From 3bd09f50d40dbe345dd446661fba7d521515f067 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Tue, 15 Aug 2023 19:02:26 +0200 Subject: [PATCH 01/12] fix: Use weak references to elements inside of interceptor objects --- .../pagefactory/AppiumFieldDecorator.java | 10 +++-- .../pagefactory/ElementInterceptor.java | 3 +- .../pagefactory/WidgetInterceptor.java | 13 +++--- .../pagefactory/WidgetListInterceptor.java | 40 ++++++++++++++----- .../InterceptorOfASingleElement.java | 7 ++-- 5 files changed, 48 insertions(+), 25 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 b1119f34e..fce490767 100644 --- a/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java +++ b/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java @@ -33,6 +33,7 @@ import org.openqa.selenium.support.pagefactory.FieldDecorator; import javax.annotation.Nullable; +import java.lang.ref.WeakReference; import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.ParameterizedType; @@ -63,7 +64,7 @@ public class AppiumFieldDecorator implements FieldDecorator { private static final List> availableElementClasses = ImmutableList.of(WebElement.class, RemoteWebElement.class); public static final Duration DEFAULT_WAITING_TIMEOUT = ofSeconds(1); - private final WebDriver webDriver; + private final WeakReference webDriver; private final DefaultFieldDecorator defaultElementFieldDecoracor; private final AppiumElementLocatorFactory widgetLocatorFactory; private final String platform; @@ -79,10 +80,11 @@ public class AppiumFieldDecorator implements FieldDecorator { * @param duration is a desired duration of the waiting for an element presence. */ public AppiumFieldDecorator(SearchContext context, Duration duration) { - this.webDriver = unpackWebDriverFromSearchContext(context); + this.webDriver = new WeakReference<>(unpackWebDriverFromSearchContext(context)); + WebDriver wd = webDriver.get(); - if (this.webDriver instanceof HasCapabilities) { - Capabilities caps = ((HasCapabilities) this.webDriver).getCapabilities(); + if (wd instanceof HasCapabilities) { + Capabilities caps = ((HasCapabilities) wd).getCapabilities(); this.platform = CapabilityHelpers.getCapability(caps, CapabilityType.PLATFORM_NAME, String.class); this.automation = CapabilityHelpers.getCapability(caps, MobileCapabilityType.AUTOMATION_NAME, String.class); } else { diff --git a/src/main/java/io/appium/java_client/pagefactory/ElementInterceptor.java b/src/main/java/io/appium/java_client/pagefactory/ElementInterceptor.java index 5047fcc11..82b61990b 100644 --- a/src/main/java/io/appium/java_client/pagefactory/ElementInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/ElementInterceptor.java @@ -21,6 +21,7 @@ import org.openqa.selenium.WebElement; import org.openqa.selenium.support.pagefactory.ElementLocator; +import java.lang.ref.WeakReference; import java.lang.reflect.Method; import static io.appium.java_client.pagefactory.ThrowableUtil.extractReadableException; @@ -30,7 +31,7 @@ */ public class ElementInterceptor extends InterceptorOfASingleElement { - public ElementInterceptor(ElementLocator locator, WebDriver driver) { + public ElementInterceptor(ElementLocator locator, WeakReference driver) { super(locator, driver); } 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 9a9d1133d..02112964f 100644 --- a/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java @@ -24,6 +24,7 @@ import org.openqa.selenium.support.PageFactory; import javax.annotation.Nullable; +import java.lang.ref.WeakReference; import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.lang.reflect.Modifier; @@ -40,16 +41,16 @@ public class WidgetInterceptor extends InterceptorOfASingleElement { private final Map> instantiationMap; private final Map cachedInstances = new HashMap<>(); private final Duration duration; - private WebElement cachedElement; + private WeakReference cachedElement; /** * Proxy interceptor class for widgets. */ public WidgetInterceptor( CacheableLocator locator, - WebDriver driver, + WeakReference driver, @Nullable - WebElement cachedElement, + WeakReference cachedElement, Map> instantiationMap, Duration duration ) { @@ -62,11 +63,11 @@ public WidgetInterceptor( @Override protected Object getObject(WebElement element, Method method, Object[] args) throws Throwable { ContentType type = getCurrentContentType(element); - if (cachedElement == null + if (cachedElement == null || cachedElement.get() == null || (locator != null && !((CacheableLocator) locator).isLookUpCached()) || cachedInstances.isEmpty() ) { - cachedElement = element; + cachedElement = new WeakReference<>(element); Constructor constructor = instantiationMap.get(type); Class clazz = constructor.getDeclaringClass(); @@ -92,7 +93,7 @@ protected Object getObject(WebElement element, Method method, Object[] args) thr @Override public Object call(Object obj, Method method, Object[] args, Callable original) throws Throwable { return locator == null - ? getObject(cachedElement, method, args) + ? getObject(cachedElement.get(), method, args) : super.call(obj, method, args, original); } } 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 136ce8e4d..f1ce14e59 100644 --- a/src/main/java/io/appium/java_client/pagefactory/WidgetListInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/WidgetListInterceptor.java @@ -22,12 +22,15 @@ import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; +import java.lang.ref.WeakReference; import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.time.Duration; import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.stream.Collectors; import static io.appium.java_client.pagefactory.ThrowableUtil.extractReadableException; import static io.appium.java_client.pagefactory.utils.ProxyFactory.getEnhancedProxy; @@ -36,18 +39,22 @@ public class WidgetListInterceptor extends InterceptorOfAListOfElements { private final Map> instantiationMap; - private final List cachedWidgets = new ArrayList<>(); + private final List> cachedWidgets = new ArrayList<>(); private final Class declaredType; private final Duration duration; - private final WebDriver driver; - private List cachedElements; + private final WeakReference driver; + private List> cachedElements; /** * Proxy interceptor class for lists of widgets. */ - public WidgetListInterceptor(CacheableLocator locator, WebDriver driver, - Map> instantiationMap, - Class declaredType, Duration duration) { + public WidgetListInterceptor( + CacheableLocator locator, + WeakReference driver, + Map> instantiationMap, + Class declaredType, + Duration duration + ) { super(locator); this.instantiationMap = instantiationMap; this.declaredType = declaredType; @@ -58,23 +65,34 @@ public WidgetListInterceptor(CacheableLocator locator, WebDriver driver, @Override protected Object getObject(List elements, Method method, Object[] args) throws Throwable { if (cachedElements == null || (locator != null && !((CacheableLocator) locator).isLookUpCached())) { - cachedElements = elements; + cachedElements = elements.stream().map(WeakReference::new).collect(Collectors.toList()); cachedWidgets.clear(); ContentType type = null; - for (WebElement element : cachedElements) { + for (WeakReference elementReference : cachedElements) { + WebElement element = elementReference.get(); + if (element == null) { + continue; + } type = ofNullable(type).orElseGet(() -> getCurrentContentType(element)); Class[] params = new Class[] {instantiationMap.get(type).getParameterTypes()[0]}; - cachedWidgets.add( + cachedWidgets.add(new WeakReference<>( getEnhancedProxy( declaredType, params, new Object[] {element}, - new WidgetInterceptor(null, driver, element, instantiationMap, duration) + new WidgetInterceptor(null, driver, elementReference, instantiationMap, duration) ) + ) ); } } try { - return method.invoke(cachedWidgets, args); + return method.invoke( + cachedWidgets.stream() + .map(WeakReference::get) + .filter(Objects::nonNull) + .collect(Collectors.toList()), + args + ); } catch (Throwable t) { throw extractReadableException(t); } diff --git a/src/main/java/io/appium/java_client/pagefactory/interceptors/InterceptorOfASingleElement.java b/src/main/java/io/appium/java_client/pagefactory/interceptors/InterceptorOfASingleElement.java index 7ac12b3d6..e4843f6e9 100644 --- a/src/main/java/io/appium/java_client/pagefactory/interceptors/InterceptorOfASingleElement.java +++ b/src/main/java/io/appium/java_client/pagefactory/interceptors/InterceptorOfASingleElement.java @@ -22,14 +22,15 @@ import org.openqa.selenium.WrapsDriver; import org.openqa.selenium.support.pagefactory.ElementLocator; +import java.lang.ref.WeakReference; import java.lang.reflect.Method; import java.util.concurrent.Callable; public abstract class InterceptorOfASingleElement implements MethodCallListener { protected final ElementLocator locator; - protected final WebDriver driver; + private final WeakReference driver; - public InterceptorOfASingleElement(ElementLocator locator, WebDriver driver) { + public InterceptorOfASingleElement(ElementLocator locator, WeakReference driver) { this.locator = locator; this.driver = driver; } @@ -48,7 +49,7 @@ public Object call(Object obj, Method method, Object[] args, Callable origina if (WrapsDriver.class.isAssignableFrom(method.getDeclaringClass()) && method.getName().equals("getWrappedDriver")) { - return driver; + return driver.get(); } WebElement realElement = locator.findElement(); From 0c6db4ce0e4ed6045716c93c901883414f08e7ba Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Tue, 15 Aug 2023 19:13:43 +0200 Subject: [PATCH 02/12] naming --- .../pagefactory/WidgetListInterceptor.java | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) 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 f1ce14e59..609ca3057 100644 --- a/src/main/java/io/appium/java_client/pagefactory/WidgetListInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/WidgetListInterceptor.java @@ -39,11 +39,11 @@ public class WidgetListInterceptor extends InterceptorOfAListOfElements { private final Map> instantiationMap; - private final List> cachedWidgets = new ArrayList<>(); + private final List> cachedWidgetReferences = new ArrayList<>(); private final Class declaredType; private final Duration duration; private final WeakReference driver; - private List> cachedElements; + private List> cachedElementReferences; /** * Proxy interceptor class for lists of widgets. @@ -64,19 +64,23 @@ public WidgetListInterceptor( @Override protected Object getObject(List elements, Method method, Object[] args) throws Throwable { - if (cachedElements == null || (locator != null && !((CacheableLocator) locator).isLookUpCached())) { - cachedElements = elements.stream().map(WeakReference::new).collect(Collectors.toList()); - cachedWidgets.clear(); + if (cachedElementReferences == null + || (locator != null && !((CacheableLocator) locator).isLookUpCached())) { + cachedElementReferences = elements.stream() + .filter(Objects::nonNull) + .map(WeakReference::new) + .collect(Collectors.toList()); + cachedWidgetReferences.clear(); ContentType type = null; - for (WeakReference elementReference : cachedElements) { + for (WeakReference elementReference : cachedElementReferences) { WebElement element = elementReference.get(); if (element == null) { continue; } type = ofNullable(type).orElseGet(() -> getCurrentContentType(element)); Class[] params = new Class[] {instantiationMap.get(type).getParameterTypes()[0]}; - cachedWidgets.add(new WeakReference<>( + cachedWidgetReferences.add(new WeakReference<>( getEnhancedProxy( declaredType, params, new Object[] {element}, new WidgetInterceptor(null, driver, elementReference, instantiationMap, duration) @@ -87,7 +91,7 @@ protected Object getObject(List elements, Method method, Object[] ar } try { return method.invoke( - cachedWidgets.stream() + cachedWidgetReferences.stream() .map(WeakReference::get) .filter(Objects::nonNull) .collect(Collectors.toList()), From 22cead243a457c032f383a22c1520c14fb0fd53b Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Tue, 15 Aug 2023 19:35:38 +0200 Subject: [PATCH 03/12] Rename --- .../pagefactory/AppiumFieldDecorator.java | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 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 fce490767..c360ae132 100644 --- a/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java +++ b/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java @@ -64,8 +64,8 @@ public class AppiumFieldDecorator implements FieldDecorator { private static final List> availableElementClasses = ImmutableList.of(WebElement.class, RemoteWebElement.class); public static final Duration DEFAULT_WAITING_TIMEOUT = ofSeconds(1); - private final WeakReference webDriver; - private final DefaultFieldDecorator defaultElementFieldDecoracor; + private final WeakReference webDriverReference; + private final DefaultFieldDecorator defaultElementFieldDecorator; private final AppiumElementLocatorFactory widgetLocatorFactory; private final String platform; private final String automation; @@ -80,9 +80,8 @@ public class AppiumFieldDecorator implements FieldDecorator { * @param duration is a desired duration of the waiting for an element presence. */ public AppiumFieldDecorator(SearchContext context, Duration duration) { - this.webDriver = new WeakReference<>(unpackWebDriverFromSearchContext(context)); - WebDriver wd = webDriver.get(); - + WebDriver wd = unpackWebDriverFromSearchContext(context); + this.webDriverReference = wd == null ? null : new WeakReference<>(wd); if (wd instanceof HasCapabilities) { Capabilities caps = ((HasCapabilities) wd).getCapabilities(); this.platform = CapabilityHelpers.getCapability(caps, CapabilityType.PLATFORM_NAME, String.class); @@ -94,7 +93,7 @@ public AppiumFieldDecorator(SearchContext context, Duration duration) { this.duration = duration; - defaultElementFieldDecoracor = new DefaultFieldDecorator( + defaultElementFieldDecorator = new DefaultFieldDecorator( new AppiumElementLocatorFactory(context, duration, new DefaultElementByBuilder(platform, automation)) ) { @Override @@ -146,7 +145,7 @@ public AppiumFieldDecorator(SearchContext context) { * @return a field value or null. */ public Object decorate(ClassLoader ignored, Field field) { - Object result = defaultElementFieldDecoracor.decorate(ignored, field); + Object result = defaultElementFieldDecorator.decorate(ignored, field); return result == null ? decorateWidget(field) : result; } @@ -193,7 +192,7 @@ private Object decorateWidget(Field field) { if (isAlist) { return getEnhancedProxy( ArrayList.class, - new WidgetListInterceptor(locator, webDriver, map, widgetType, duration) + new WidgetListInterceptor(locator, webDriverReference, map, widgetType, duration) ); } @@ -202,12 +201,12 @@ private Object decorateWidget(Field field) { widgetType, new Class[]{constructor.getParameterTypes()[0]}, new Object[]{proxyForAnElement(locator)}, - new WidgetInterceptor(locator, webDriver, null, map, duration) + new WidgetInterceptor(locator, webDriverReference, null, map, duration) ); } private WebElement proxyForAnElement(ElementLocator locator) { - ElementInterceptor elementInterceptor = new ElementInterceptor(locator, webDriver); + ElementInterceptor elementInterceptor = new ElementInterceptor(locator, webDriverReference); return getEnhancedProxy(RemoteWebElement.class, elementInterceptor); } } From bb81bacd87759c429120c08fef28e9c13ec960b3 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Tue, 15 Aug 2023 20:08:36 +0200 Subject: [PATCH 04/12] Fix constructor arg --- .../pagefactory/WidgetInterceptor.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) 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 02112964f..bb6067784 100644 --- a/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java @@ -41,7 +41,7 @@ public class WidgetInterceptor extends InterceptorOfASingleElement { private final Map> instantiationMap; private final Map cachedInstances = new HashMap<>(); private final Duration duration; - private WeakReference cachedElement; + private WeakReference cachedElementReference; /** * Proxy interceptor class for widgets. @@ -50,12 +50,12 @@ public WidgetInterceptor( CacheableLocator locator, WeakReference driver, @Nullable - WeakReference cachedElement, + WeakReference cachedElementReference, Map> instantiationMap, Duration duration ) { super(locator, driver); - this.cachedElement = cachedElement; + this.cachedElementReference = cachedElementReference; this.instantiationMap = instantiationMap; this.duration = duration; } @@ -63,11 +63,11 @@ public WidgetInterceptor( @Override protected Object getObject(WebElement element, Method method, Object[] args) throws Throwable { ContentType type = getCurrentContentType(element); - if (cachedElement == null || cachedElement.get() == null + if (cachedElementReference == null || cachedElementReference.get() == null || (locator != null && !((CacheableLocator) locator).isLookUpCached()) || cachedInstances.isEmpty() ) { - cachedElement = new WeakReference<>(element); + cachedElementReference = new WeakReference<>(element); Constructor constructor = instantiationMap.get(type); Class clazz = constructor.getDeclaringClass(); @@ -78,7 +78,7 @@ protected Object getObject(WebElement element, Method method, Object[] args) thr ); } - Widget widget = constructor.newInstance(cachedElement); + Widget widget = constructor.newInstance(element); cachedInstances.put(type, widget); PageFactory.initElements(new AppiumFieldDecorator(widget, duration), widget); } @@ -92,8 +92,9 @@ protected Object getObject(WebElement element, Method method, Object[] args) thr @Override public Object call(Object obj, Method method, Object[] args, Callable original) throws Throwable { - return locator == null - ? getObject(cachedElement.get(), method, args) + WebElement element = cachedElementReference.get(); + return locator == null && element != null + ? getObject(element, method, args) : super.call(obj, method, args, original); } } From 38eba319f0c52ecc81dfec997130827977f57a32 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Tue, 15 Aug 2023 20:10:25 +0200 Subject: [PATCH 05/12] Extra space --- .../io/appium/java_client/pagefactory/WidgetInterceptor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 bb6067784..033c30a07 100644 --- a/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java @@ -74,7 +74,7 @@ protected Object getObject(WebElement element, Method method, Object[] args) thr if (Modifier.isAbstract(clazz.getModifiers())) { throw new InstantiationException( - String.format("%s is abstract so it cannot be instantiated", clazz.getName()) + String.format("%s is abstract so it cannot be instantiated", clazz.getName()) ); } From 7e1749f0790fffcf176d7a11f1d9c0c61f5a69c8 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Tue, 15 Aug 2023 22:43:41 +0200 Subject: [PATCH 06/12] fix getting element --- .../io/appium/java_client/pagefactory/WidgetInterceptor.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 033c30a07..ec5bae0df 100644 --- a/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java @@ -35,6 +35,7 @@ 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; public class WidgetInterceptor extends InterceptorOfASingleElement { @@ -92,7 +93,7 @@ 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 = cachedElementReference.get(); + WebElement element = ofNullable(cachedElementReference).map(WeakReference::get).orElse(null); return locator == null && element != null ? getObject(element, method, args) : super.call(obj, method, args, original); From 2a13a57485694ab495ad4f34e3d6caa60a20bf49 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Tue, 15 Aug 2023 23:27:17 +0200 Subject: [PATCH 07/12] Use weak hash map --- .../java_client/pagefactory/WidgetInterceptor.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) 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 ec5bae0df..39b123ff1 100644 --- a/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java @@ -31,6 +31,7 @@ import java.time.Duration; import java.util.HashMap; import java.util.Map; +import java.util.WeakHashMap; import java.util.concurrent.Callable; import static io.appium.java_client.pagefactory.ThrowableUtil.extractReadableException; @@ -40,7 +41,7 @@ public class WidgetInterceptor extends InterceptorOfASingleElement { private final Map> instantiationMap; - private final Map cachedInstances = new HashMap<>(); + private final Map> cachedInstances = new WeakHashMap<>(); private final Duration duration; private WeakReference cachedElementReference; @@ -80,12 +81,14 @@ protected Object getObject(WebElement element, Method method, Object[] args) thr } Widget widget = constructor.newInstance(element); - cachedInstances.put(type, widget); + Map typeMap = ofNullable(cachedInstances.get(element)).orElseGet(HashMap::new); + typeMap.put(type, widget); + cachedInstances.put(element, typeMap); PageFactory.initElements(new AppiumFieldDecorator(widget, duration), widget); } try { method.setAccessible(true); - return method.invoke(cachedInstances.get(type), args); + return method.invoke(cachedInstances.get(cachedElementReference.get()).get(type), args); } catch (Throwable t) { throw extractReadableException(t); } From 4ba98918a5edebf39345fa82af29f5c782ee18ee Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Wed, 16 Aug 2023 21:12:42 +0200 Subject: [PATCH 08/12] Make weak ref to context --- .../pagefactory/AppiumElementLocator.java | 96 +++++++++++++------ .../AppiumElementLocatorFactory.java | 47 ++++++--- .../pagefactory/AppiumFieldDecorator.java | 73 +++++++++++++- .../pagefactory/WidgetInterceptor.java | 9 +- .../pagefactory/WidgetListInterceptor.java | 2 + .../InterceptorOfAListOfElements.java | 5 +- .../InterceptorOfASingleElement.java | 17 +++- 7 files changed, 199 insertions(+), 50 deletions(-) diff --git a/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocator.java b/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocator.java index 5208add47..ba2496613 100644 --- a/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocator.java +++ b/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocator.java @@ -27,9 +27,11 @@ import org.openqa.selenium.WebElement; import org.openqa.selenium.support.ui.FluentWait; +import java.lang.ref.WeakReference; import java.time.Duration; import java.util.ArrayList; import java.util.List; +import java.util.Optional; import java.util.function.Function; import java.util.function.Supplier; @@ -46,10 +48,36 @@ class AppiumElementLocator implements CacheableLocator { private final boolean shouldCache; private final By by; private final Duration duration; + private final WeakReference searchContextReference; private final SearchContext searchContext; + private WebElement cachedElement; private List cachedElementList; + /** + * Creates a new mobile element locator. It instantiates {@link WebElement} + * using @AndroidFindBy (-s), @iOSFindBy (-s) and @FindBy (-s) annotation + * sets + * + * @param searchContextReference The context reference to use when finding the element + * @param by a By locator strategy + * @param shouldCache is the flag that signalizes that elements which + * are found once should be cached + * @param duration timeout parameter for the element to be found + */ + public AppiumElementLocator( + WeakReference searchContextReference, + By by, + boolean shouldCache, + Duration duration + ) { + this.searchContextReference = searchContextReference; + this.searchContext = null; + this.shouldCache = shouldCache; + this.duration = duration; + this.by = by; + } + /** * Creates a new mobile element locator. It instantiates {@link WebElement} * using @AndroidFindBy (-s), @iOSFindBy (-s) and @FindBy (-s) annotation @@ -61,15 +89,29 @@ class AppiumElementLocator implements CacheableLocator { * are found once should be cached * @param duration timeout parameter for the element to be found */ - - public AppiumElementLocator(SearchContext searchContext, By by, boolean shouldCache, - Duration duration) { + public AppiumElementLocator( + SearchContext searchContext, + By by, + boolean shouldCache, + Duration duration + ) { + this.searchContextReference = null; this.searchContext = searchContext; this.shouldCache = shouldCache; this.duration = duration; this.by = by; } + private Optional getSearchContext() { + if (searchContext != null) { + return Optional.of(searchContext); + } + if (searchContextReference != null) { + return Optional.ofNullable(searchContextReference.get()); + } + return Optional.empty(); + } + /** * This methods makes sets some settings of the {@link By} according to * the given instance of {@link SearchContext}. If there is some {@link ContentMappedBy} @@ -85,8 +127,7 @@ private static By getBy(By currentBy, SearchContext currentContent) { return currentBy; } - return ContentMappedBy.class.cast(currentBy) - .useContent(getCurrentContentType(currentContent)); + return ((ContentMappedBy) currentBy).useContent(getCurrentContentType(currentContent)); } private T waitFor(Supplier supplier) { @@ -98,8 +139,7 @@ private T waitFor(Supplier supplier) { return wait.until(function); } catch (TimeoutException e) { if (function.foundStaleElementReferenceException != null) { - throw StaleElementReferenceException - .class.cast(function.foundStaleElementReferenceException); + throw (StaleElementReferenceException) function.foundStaleElementReferenceException; } throw e; } @@ -113,10 +153,15 @@ public WebElement findElement() { return cachedElement; } + SearchContext searchContext = getSearchContext() + .orElseThrow(() -> new IllegalStateException( + String.format("The element %s is not locatable anymore " + + "because its context has been garbage collected", by) + )); + By bySearching = getBy(this.by, searchContext); try { - WebElement result = waitFor(() -> - searchContext.findElement(bySearching)); + WebElement result = waitFor(() -> searchContext.findElement(bySearching)); if (shouldCache) { cachedElement = result; } @@ -134,12 +179,17 @@ public List findElements() { return cachedElementList; } + SearchContext searchContext = getSearchContext() + .orElseThrow(() -> new IllegalStateException( + String.format("Elements %s are not locatable anymore " + + "because their context has been garbage collected", by) + )); + List result; try { result = waitFor(() -> { - List list = searchContext - .findElements(getBy(by, searchContext)); - return list.size() > 0 ? list : null; + List list = searchContext.findElements(getBy(by, searchContext)); + return list.isEmpty() ? null : list; }); } catch (TimeoutException | StaleElementReferenceException e) { result = new ArrayList<>(); @@ -171,30 +221,22 @@ public T apply(Supplier supplier) { return supplier.get(); } catch (Throwable e) { boolean isRootCauseStaleElementReferenceException = false; - Throwable shouldBeThrown; boolean isRootCauseInvalidSelector = isInvalidSelectorRootCause(e); - if (!isRootCauseInvalidSelector) { isRootCauseStaleElementReferenceException = isStaleElementReferenceException(e); } - if (isRootCauseStaleElementReferenceException) { foundStaleElementReferenceException = extractReadableException(e); } + if (isRootCauseInvalidSelector || isRootCauseStaleElementReferenceException) { + return null; + } - if (!isRootCauseInvalidSelector & !isRootCauseStaleElementReferenceException) { - shouldBeThrown = extractReadableException(e); - if (shouldBeThrown != null) { - if (NoSuchElementException.class.equals(shouldBeThrown.getClass())) { - throw NoSuchElementException.class.cast(shouldBeThrown); - } else { - throw new WebDriverException(shouldBeThrown); - } - } else { - throw new WebDriverException(e); - } + Throwable excToThrow = extractReadableException(e); + if (excToThrow instanceof WebDriverException) { + throw (WebDriverException) excToThrow; } else { - return null; + throw new WebDriverException(excToThrow); } } } diff --git a/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocatorFactory.java b/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocatorFactory.java index 0fc8e3d38..7f42b8a51 100644 --- a/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocatorFactory.java +++ b/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocatorFactory.java @@ -23,6 +23,7 @@ import org.openqa.selenium.SearchContext; import javax.annotation.Nullable; +import java.lang.ref.WeakReference; import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Field; import java.time.Duration; @@ -32,6 +33,7 @@ public class AppiumElementLocatorFactory implements CacheableElementLocatorFactory { private final SearchContext searchContext; + private final WeakReference searchContextReference; private final Duration duration; private final AppiumByBuilder builder; @@ -39,22 +41,46 @@ public class AppiumElementLocatorFactory implements CacheableElementLocatorFacto * Creates a new mobile element locator factory. * * @param searchContext The context to use when finding the element - * @param duration timeout parameters for the elements to be found - * @param builder is handler of Appium-specific page object annotations + * @param duration timeout parameters for the elements to be found + * @param builder is handler of Appium-specific page object annotations */ - public AppiumElementLocatorFactory(SearchContext searchContext, Duration duration, - AppiumByBuilder builder) { + public AppiumElementLocatorFactory( + SearchContext searchContext, + Duration duration, + AppiumByBuilder builder + ) { this.searchContext = searchContext; + this.searchContextReference = null; this.duration = duration; this.builder = builder; } - public @Nullable CacheableLocator createLocator(Field field) { - return this.createLocator((AnnotatedElement) field); + /** + * Creates a new mobile element locator factory. + * + * @param searchContextReference The context reference to use when finding the element + * @param duration timeout parameters for the elements to be found + * @param builder is handler of Appium-specific page object annotations + */ + public AppiumElementLocatorFactory( + WeakReference searchContextReference, + Duration duration, + AppiumByBuilder builder + ) { + this.searchContextReference = searchContextReference; + this.searchContext = null; + this.duration = duration; + this.builder = builder; } + @Nullable @Override + public CacheableLocator createLocator(Field field) { + return this.createLocator((AnnotatedElement) field); + } + @Nullable + @Override public CacheableLocator createLocator(AnnotatedElement annotatedElement) { Duration customDuration; if (annotatedElement.isAnnotationPresent(WithTimeout.class)) { @@ -63,14 +89,13 @@ public CacheableLocator createLocator(AnnotatedElement annotatedElement) { } else { customDuration = duration; } - builder.setAnnotated(annotatedElement); By byResult = builder.buildBy(); - return ofNullable(byResult) - .map(by -> new AppiumElementLocator(searchContext, by, builder.isLookupCached(), customDuration)) + .map(by -> searchContextReference != null + ? new AppiumElementLocator(searchContextReference, by, builder.isLookupCached(), customDuration) + : new AppiumElementLocator(searchContext, by, builder.isLookupCached(), customDuration) + ) .orElse(null); } - - } 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 c360ae132..aab041451 100644 --- a/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java +++ b/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java @@ -61,8 +61,10 @@ */ public class AppiumFieldDecorator implements FieldDecorator { - private static final List> availableElementClasses = ImmutableList.of(WebElement.class, - RemoteWebElement.class); + private static final List> availableElementClasses = ImmutableList.of( + WebElement.class, + RemoteWebElement.class + ); public static final Duration DEFAULT_WAITING_TIMEOUT = ofSeconds(1); private final WeakReference webDriverReference; private final DefaultFieldDecorator defaultElementFieldDecorator; @@ -137,6 +139,73 @@ public AppiumFieldDecorator(SearchContext context) { this(context, DEFAULT_WAITING_TIMEOUT); } + /** + * Creates field decorator based on {@link SearchContext} and timeout {@code duration}. + * + * @param contextReference reference to {@link SearchContext} + * It may be the instance of {@link WebDriver} or {@link WebElement} or + * {@link Widget} or some other user's extension/implementation. + * @param duration is a desired duration of the waiting for an element presence. + */ + public AppiumFieldDecorator(WeakReference contextReference, Duration duration) { + WebDriver wd = unpackWebDriverFromSearchContext(contextReference.get()); + this.webDriverReference = wd == null ? null : new WeakReference<>(wd); + if (wd instanceof HasCapabilities) { + Capabilities caps = ((HasCapabilities) wd).getCapabilities(); + this.platform = CapabilityHelpers.getCapability(caps, CapabilityType.PLATFORM_NAME, String.class); + this.automation = CapabilityHelpers.getCapability(caps, MobileCapabilityType.AUTOMATION_NAME, String.class); + } else { + this.platform = null; + this.automation = null; + } + + this.duration = duration; + + defaultElementFieldDecorator = new DefaultFieldDecorator( + new AppiumElementLocatorFactory(contextReference, duration, new DefaultElementByBuilder(platform, automation)) + ) { + @Override + protected WebElement proxyForLocator(ClassLoader ignored, ElementLocator locator) { + return proxyForAnElement(locator); + } + + @Override + protected List proxyForListLocator(ClassLoader ignored, ElementLocator locator) { + ElementListInterceptor elementInterceptor = new ElementListInterceptor(locator); + //noinspection unchecked + return getEnhancedProxy(ArrayList.class, elementInterceptor); + } + + @Override + protected boolean isDecoratableList(Field field) { + if (!List.class.isAssignableFrom(field.getType())) { + return false; + } + + Type genericType = field.getGenericType(); + if (!(genericType instanceof ParameterizedType)) { + return false; + } + + Type listType = ((ParameterizedType) genericType).getActualTypeArguments()[0]; + List bounds = (listType instanceof TypeVariable) + ? Arrays.asList(((TypeVariable) listType).getBounds()) + : Collections.emptyList(); + return availableElementClasses.stream() + .anyMatch((webElClass) -> webElClass.equals(listType) || bounds.contains(webElClass)); + } + }; + + widgetLocatorFactory = new AppiumElementLocatorFactory( + contextReference, duration, new WidgetByBuilder(platform, automation) + ); + } + + public AppiumFieldDecorator(WeakReference contextReference) { + this(contextReference, DEFAULT_WAITING_TIMEOUT); + } + + /** * Decorated page object {@code field}. * 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 39b123ff1..7532d6784 100644 --- a/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java @@ -49,14 +49,15 @@ public class WidgetInterceptor extends InterceptorOfASingleElement { * Proxy interceptor class for widgets. */ public WidgetInterceptor( + @Nullable CacheableLocator locator, - WeakReference driver, + WeakReference driverReference, @Nullable WeakReference cachedElementReference, Map> instantiationMap, Duration duration ) { - super(locator, driver); + super(locator, driverReference); this.cachedElementReference = cachedElementReference; this.instantiationMap = instantiationMap; this.duration = duration; @@ -66,8 +67,8 @@ public WidgetInterceptor( protected Object getObject(WebElement element, Method method, Object[] args) throws Throwable { ContentType type = getCurrentContentType(element); if (cachedElementReference == null || cachedElementReference.get() == null - || (locator != null && !((CacheableLocator) locator).isLookUpCached()) || cachedInstances.isEmpty() + || (locator != null && !((CacheableLocator) locator).isLookUpCached()) ) { cachedElementReference = new WeakReference<>(element); @@ -84,7 +85,7 @@ protected Object getObject(WebElement element, Method method, Object[] args) thr Map typeMap = ofNullable(cachedInstances.get(element)).orElseGet(HashMap::new); typeMap.put(type, widget); cachedInstances.put(element, typeMap); - PageFactory.initElements(new AppiumFieldDecorator(widget, duration), widget); + PageFactory.initElements(new AppiumFieldDecorator(new WeakReference<>(widget), duration), widget); } try { method.setAccessible(true); 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 609ca3057..aa4c09b95 100644 --- a/src/main/java/io/appium/java_client/pagefactory/WidgetListInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/WidgetListInterceptor.java @@ -22,6 +22,7 @@ import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; +import javax.annotation.Nullable; import java.lang.ref.WeakReference; import java.lang.reflect.Constructor; import java.lang.reflect.Method; @@ -49,6 +50,7 @@ public class WidgetListInterceptor extends InterceptorOfAListOfElements { * Proxy interceptor class for lists of widgets. */ public WidgetListInterceptor( + @Nullable CacheableLocator locator, WeakReference driver, Map> instantiationMap, diff --git a/src/main/java/io/appium/java_client/pagefactory/interceptors/InterceptorOfAListOfElements.java b/src/main/java/io/appium/java_client/pagefactory/interceptors/InterceptorOfAListOfElements.java index d276ce616..62e1442aa 100644 --- a/src/main/java/io/appium/java_client/pagefactory/interceptors/InterceptorOfAListOfElements.java +++ b/src/main/java/io/appium/java_client/pagefactory/interceptors/InterceptorOfAListOfElements.java @@ -20,6 +20,7 @@ import org.openqa.selenium.WebElement; import org.openqa.selenium.support.pagefactory.ElementLocator; +import javax.annotation.Nullable; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.List; @@ -28,7 +29,7 @@ public abstract class InterceptorOfAListOfElements implements MethodCallListener { protected final ElementLocator locator; - public InterceptorOfAListOfElements(ElementLocator locator) { + public InterceptorOfAListOfElements(@Nullable ElementLocator locator) { this.locator = locator; } @@ -38,7 +39,7 @@ protected abstract Object getObject( @Override public Object call(Object obj, Method method, Object[] args, Callable original) throws Throwable { - if (Object.class.equals(method.getDeclaringClass())) { + if (locator == null || Object.class.equals(method.getDeclaringClass())) { return original.call(); } diff --git a/src/main/java/io/appium/java_client/pagefactory/interceptors/InterceptorOfASingleElement.java b/src/main/java/io/appium/java_client/pagefactory/interceptors/InterceptorOfASingleElement.java index e4843f6e9..7eea82233 100644 --- a/src/main/java/io/appium/java_client/pagefactory/interceptors/InterceptorOfASingleElement.java +++ b/src/main/java/io/appium/java_client/pagefactory/interceptors/InterceptorOfASingleElement.java @@ -22,23 +22,32 @@ import org.openqa.selenium.WrapsDriver; import org.openqa.selenium.support.pagefactory.ElementLocator; +import javax.annotation.Nullable; import java.lang.ref.WeakReference; import java.lang.reflect.Method; import java.util.concurrent.Callable; public abstract class InterceptorOfASingleElement implements MethodCallListener { protected final ElementLocator locator; - private final WeakReference driver; + private final WeakReference driverReference; - public InterceptorOfASingleElement(ElementLocator locator, WeakReference driver) { + public InterceptorOfASingleElement( + @Nullable + ElementLocator locator, + WeakReference driverReference + ) { this.locator = locator; - this.driver = driver; + this.driverReference = driverReference; } protected abstract Object getObject(WebElement element, Method method, Object[] args) throws Throwable; @Override public Object call(Object obj, Method method, Object[] args, Callable original) throws Throwable { + if (locator == null) { + return original.call(); + } + if (method.getName().equals("toString") && args.length == 0) { return locator.toString(); } @@ -49,7 +58,7 @@ public Object call(Object obj, Method method, Object[] args, Callable origina if (WrapsDriver.class.isAssignableFrom(method.getDeclaringClass()) && method.getName().equals("getWrappedDriver")) { - return driver.get(); + return driverReference.get(); } WebElement realElement = locator.findElement(); From 6a228f85929c491921c03f6366c7d1bdbe137090 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Wed, 16 Aug 2023 21:16:19 +0200 Subject: [PATCH 09/12] fix lint --- .../java_client/pagefactory/AppiumElementLocator.java | 8 ++++---- .../java_client/pagefactory/AppiumFieldDecorator.java | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocator.java b/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocator.java index ba2496613..7dc44701b 100644 --- a/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocator.java +++ b/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocator.java @@ -155,8 +155,8 @@ public WebElement findElement() { SearchContext searchContext = getSearchContext() .orElseThrow(() -> new IllegalStateException( - String.format("The element %s is not locatable anymore " + - "because its context has been garbage collected", by) + String.format("The element %s is not locatable anymore " + + "because its context has been garbage collected", by) )); By bySearching = getBy(this.by, searchContext); @@ -181,8 +181,8 @@ public List findElements() { SearchContext searchContext = getSearchContext() .orElseThrow(() -> new IllegalStateException( - String.format("Elements %s are not locatable anymore " + - "because their context has been garbage collected", by) + String.format("Elements %s are not locatable anymore " + + "because their context has been garbage collected", by) )); List result; 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 aab041451..dc4fecb1c 100644 --- a/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java +++ b/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java @@ -162,7 +162,9 @@ public AppiumFieldDecorator(WeakReference contextReference, Durat this.duration = duration; defaultElementFieldDecorator = new DefaultFieldDecorator( - new AppiumElementLocatorFactory(contextReference, duration, new DefaultElementByBuilder(platform, automation)) + new AppiumElementLocatorFactory( + contextReference, duration, new DefaultElementByBuilder(platform, automation) + ) ) { @Override protected WebElement proxyForLocator(ClassLoader ignored, ElementLocator locator) { From a06c53f99e78c863aad22428fac96ab2fa5fc9c9 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Thu, 17 Aug 2023 09:22:37 +0200 Subject: [PATCH 10/12] Store widgets without referncing --- .../pagefactory/WidgetInterceptor.java | 13 +++---- .../pagefactory/WidgetListInterceptor.java | 34 +++++++------------ 2 files changed, 18 insertions(+), 29 deletions(-) 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 7532d6784..d00c6e3c9 100644 --- a/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java @@ -31,7 +31,6 @@ import java.time.Duration; import java.util.HashMap; import java.util.Map; -import java.util.WeakHashMap; import java.util.concurrent.Callable; import static io.appium.java_client.pagefactory.ThrowableUtil.extractReadableException; @@ -41,7 +40,7 @@ public class WidgetInterceptor extends InterceptorOfASingleElement { private final Map> instantiationMap; - private final Map> cachedInstances = new WeakHashMap<>(); + private final Map cachedInstances = new HashMap<>(); private final Duration duration; private WeakReference cachedElementReference; @@ -66,8 +65,8 @@ public WidgetInterceptor( @Override protected Object getObject(WebElement element, Method method, Object[] args) throws Throwable { ContentType type = getCurrentContentType(element); - if (cachedElementReference == null || cachedElementReference.get() == null - || cachedInstances.isEmpty() + WebElement cachedElement = cachedElementReference == null ? null : cachedElementReference.get(); + if (cachedElement == null || !cachedInstances.containsKey(type) || (locator != null && !((CacheableLocator) locator).isLookUpCached()) ) { cachedElementReference = new WeakReference<>(element); @@ -82,14 +81,12 @@ protected Object getObject(WebElement element, Method method, Object[] args) thr } Widget widget = constructor.newInstance(element); - Map typeMap = ofNullable(cachedInstances.get(element)).orElseGet(HashMap::new); - typeMap.put(type, widget); - cachedInstances.put(element, typeMap); + cachedInstances.put(type, widget); PageFactory.initElements(new AppiumFieldDecorator(new WeakReference<>(widget), duration), widget); } try { method.setAccessible(true); - return method.invoke(cachedInstances.get(cachedElementReference.get()).get(type), args); + return method.invoke(cachedInstances.get(type), args); } catch (Throwable t) { throw extractReadableException(t); } 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 aa4c09b95..031a0e624 100644 --- a/src/main/java/io/appium/java_client/pagefactory/WidgetListInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/WidgetListInterceptor.java @@ -40,11 +40,11 @@ public class WidgetListInterceptor extends InterceptorOfAListOfElements { private final Map> instantiationMap; - private final List> cachedWidgetReferences = new ArrayList<>(); + private final List cachedWidgets = new ArrayList<>(); private final Class declaredType; private final Duration duration; private final WeakReference driver; - private List> cachedElementReferences; + private final List> cachedElementReferences = new ArrayList<>(); /** * Proxy interceptor class for lists of widgets. @@ -66,39 +66,31 @@ public WidgetListInterceptor( @Override protected Object getObject(List elements, Method method, Object[] args) throws Throwable { - if (cachedElementReferences == null - || (locator != null && !((CacheableLocator) locator).isLookUpCached())) { - cachedElementReferences = elements.stream() + List cachedElements = cachedElementReferences.stream() + .map(WeakReference::get) .filter(Objects::nonNull) - .map(WeakReference::new) .collect(Collectors.toList()); - cachedWidgetReferences.clear(); + if (cachedElements.size() != cachedWidgets.size() + || (locator != null && !((CacheableLocator) locator).isLookUpCached())) { + cachedWidgets.clear(); + cachedElementReferences.clear(); ContentType type = null; - for (WeakReference elementReference : cachedElementReferences) { - WebElement element = elementReference.get(); - if (element == null) { - continue; - } + for (WebElement element : elements) { type = ofNullable(type).orElseGet(() -> getCurrentContentType(element)); Class[] params = new Class[] {instantiationMap.get(type).getParameterTypes()[0]}; - cachedWidgetReferences.add(new WeakReference<>( + WeakReference elementReference = new WeakReference<>(element); + cachedWidgets.add( getEnhancedProxy( declaredType, params, new Object[] {element}, new WidgetInterceptor(null, driver, elementReference, instantiationMap, duration) ) - ) ); + cachedElementReferences.add(elementReference); } } try { - return method.invoke( - cachedWidgetReferences.stream() - .map(WeakReference::get) - .filter(Objects::nonNull) - .collect(Collectors.toList()), - args - ); + return method.invoke(cachedWidgets, args); } catch (Throwable t) { throw extractReadableException(t); } From ad12f3a3701153dc2d5640782067c2baa47eef9e Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Fri, 18 Aug 2023 10:50:35 +0200 Subject: [PATCH 11/12] Extract common code --- .../pagefactory/AppiumFieldDecorator.java | 67 ++++++------------- 1 file changed, 19 insertions(+), 48 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 dc4fecb1c..477afc282 100644 --- a/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java +++ b/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java @@ -30,6 +30,7 @@ import org.openqa.selenium.remote.RemoteWebElement; import org.openqa.selenium.support.pagefactory.DefaultFieldDecorator; import org.openqa.selenium.support.pagefactory.ElementLocator; +import org.openqa.selenium.support.pagefactory.ElementLocatorFactory; import org.openqa.selenium.support.pagefactory.FieldDecorator; import javax.annotation.Nullable; @@ -95,40 +96,9 @@ public AppiumFieldDecorator(SearchContext context, Duration duration) { this.duration = duration; - defaultElementFieldDecorator = new DefaultFieldDecorator( - new AppiumElementLocatorFactory(context, duration, new DefaultElementByBuilder(platform, automation)) - ) { - @Override - protected WebElement proxyForLocator(ClassLoader ignored, ElementLocator locator) { - return proxyForAnElement(locator); - } - - @Override - protected List proxyForListLocator(ClassLoader ignored, ElementLocator locator) { - ElementListInterceptor elementInterceptor = new ElementListInterceptor(locator); - //noinspection unchecked - return getEnhancedProxy(ArrayList.class, elementInterceptor); - } - - @Override - protected boolean isDecoratableList(Field field) { - if (!List.class.isAssignableFrom(field.getType())) { - return false; - } - - Type genericType = field.getGenericType(); - if (!(genericType instanceof ParameterizedType)) { - return false; - } - - Type listType = ((ParameterizedType) genericType).getActualTypeArguments()[0]; - List bounds = (listType instanceof TypeVariable) - ? Arrays.asList(((TypeVariable) listType).getBounds()) - : Collections.emptyList(); - return availableElementClasses.stream() - .anyMatch((webElClass) -> webElClass.equals(listType) || bounds.contains(webElClass)); - } - }; + defaultElementFieldDecorator = createFieldDecorator(new AppiumElementLocatorFactory( + context, duration, new DefaultElementByBuilder(platform, automation) + )); widgetLocatorFactory = new AppiumElementLocatorFactory( context, duration, new WidgetByBuilder(platform, automation) @@ -161,11 +131,21 @@ public AppiumFieldDecorator(WeakReference contextReference, Durat this.duration = duration; - defaultElementFieldDecorator = new DefaultFieldDecorator( - new AppiumElementLocatorFactory( - contextReference, duration, new DefaultElementByBuilder(platform, automation) - ) - ) { + defaultElementFieldDecorator = createFieldDecorator(new AppiumElementLocatorFactory( + contextReference, duration, new DefaultElementByBuilder(platform, automation) + )); + + widgetLocatorFactory = new AppiumElementLocatorFactory( + contextReference, duration, new WidgetByBuilder(platform, automation) + ); + } + + public AppiumFieldDecorator(WeakReference contextReference) { + this(contextReference, DEFAULT_WAITING_TIMEOUT); + } + + private DefaultFieldDecorator createFieldDecorator(ElementLocatorFactory factory) { + return new DefaultFieldDecorator(factory) { @Override protected WebElement proxyForLocator(ClassLoader ignored, ElementLocator locator) { return proxyForAnElement(locator); @@ -197,17 +177,8 @@ protected boolean isDecoratableList(Field field) { .anyMatch((webElClass) -> webElClass.equals(listType) || bounds.contains(webElClass)); } }; - - widgetLocatorFactory = new AppiumElementLocatorFactory( - contextReference, duration, new WidgetByBuilder(platform, automation) - ); - } - - public AppiumFieldDecorator(WeakReference contextReference) { - this(contextReference, DEFAULT_WAITING_TIMEOUT); } - /** * Decorated page object {@code field}. * From c53ed87358b2623788f6dcc91692e787f3a16f55 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Fri, 18 Aug 2023 13:40:59 +0200 Subject: [PATCH 12/12] Address review comments --- .../pagefactory/AppiumElementLocator.java | 12 ++++-------- .../pagefactory/AppiumElementLocatorFactory.java | 2 +- .../pagefactory/AppiumFieldDecorator.java | 6 +----- 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocator.java b/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocator.java index 7dc44701b..3aba5bcdc 100644 --- a/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocator.java +++ b/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocator.java @@ -65,7 +65,7 @@ class AppiumElementLocator implements CacheableLocator { * are found once should be cached * @param duration timeout parameter for the element to be found */ - public AppiumElementLocator( + AppiumElementLocator( WeakReference searchContextReference, By by, boolean shouldCache, @@ -103,13 +103,9 @@ public AppiumElementLocator( } private Optional getSearchContext() { - if (searchContext != null) { - return Optional.of(searchContext); - } - if (searchContextReference != null) { - return Optional.ofNullable(searchContextReference.get()); - } - return Optional.empty(); + return searchContext == null + ? Optional.ofNullable(searchContextReference).map(WeakReference::get) + : Optional.of(searchContext); } /** diff --git a/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocatorFactory.java b/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocatorFactory.java index 7f42b8a51..787e34e9e 100644 --- a/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocatorFactory.java +++ b/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocatorFactory.java @@ -62,7 +62,7 @@ public AppiumElementLocatorFactory( * @param duration timeout parameters for the elements to be found * @param builder is handler of Appium-specific page object annotations */ - public AppiumElementLocatorFactory( + AppiumElementLocatorFactory( WeakReference searchContextReference, Duration duration, AppiumByBuilder builder 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 477afc282..c2f11f473 100644 --- a/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java +++ b/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java @@ -117,7 +117,7 @@ public AppiumFieldDecorator(SearchContext context) { * {@link Widget} or some other user's extension/implementation. * @param duration is a desired duration of the waiting for an element presence. */ - public AppiumFieldDecorator(WeakReference contextReference, Duration duration) { + AppiumFieldDecorator(WeakReference contextReference, Duration duration) { WebDriver wd = unpackWebDriverFromSearchContext(contextReference.get()); this.webDriverReference = wd == null ? null : new WeakReference<>(wd); if (wd instanceof HasCapabilities) { @@ -140,10 +140,6 @@ contextReference, duration, new WidgetByBuilder(platform, automation) ); } - public AppiumFieldDecorator(WeakReference contextReference) { - this(contextReference, DEFAULT_WAITING_TIMEOUT); - } - private DefaultFieldDecorator createFieldDecorator(ElementLocatorFactory factory) { return new DefaultFieldDecorator(factory) { @Override