From 37a0c0a7d7acae9bbe1f651538c53dc0139e8de1 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Wed, 24 May 2023 22:51:23 +0200 Subject: [PATCH 1/7] refactor: Replace cglib with bytebuddy --- build.gradle | 1 - .../pagefactory/AppiumFieldDecorator.java | 43 +++--- .../pagefactory/ElementInterceptor.java | 7 +- .../pagefactory/ElementListInterceptor.java | 7 +- .../pagefactory/ThrowableUtil.java | 8 +- .../pagefactory/WidgetInterceptor.java | 44 +++--- .../pagefactory/WidgetListInterceptor.java | 27 ++-- .../InterceptorOfAListOfElements.java | 21 ++- .../InterceptorOfASingleElement.java | 20 ++- .../pagefactory/utils/ProxyFactory.java | 66 +++++++-- .../utils/WebDriverUnpackUtility.java | 9 +- .../io/appium/java_client/proxy/Helpers.java | 70 ++++++++-- .../appium/java_client/proxy/Interceptor.java | 17 +-- .../widget/tests/WidgetTest.java | 1 - .../tests/combined/CombinedAppTest.java | 8 +- .../tests/combined/CombinedWidgetTest.java | 9 +- .../tests/windows/AnnotatedWindowsWidget.java | 15 --- .../tests/windows/DefaultWindowsWidget.java | 35 ----- .../tests/windows/ExtendedWindowsWidget.java | 9 -- .../widget/tests/windows/WindowsApp.java | 125 ------------------ .../tests/windows/WindowsWidgetTest.java | 51 ------- 21 files changed, 219 insertions(+), 374 deletions(-) delete mode 100644 src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/windows/AnnotatedWindowsWidget.java delete mode 100644 src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/windows/DefaultWindowsWidget.java delete mode 100644 src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/windows/ExtendedWindowsWidget.java delete mode 100644 src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/windows/WindowsApp.java delete mode 100644 src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/windows/WindowsWidgetTest.java diff --git a/build.gradle b/build.gradle index 52bf70f38..2c2371e99 100644 --- a/build.gradle +++ b/build.gradle @@ -51,7 +51,6 @@ dependencies { } } implementation 'com.google.code.gson:gson:2.10.1' - implementation 'cglib:cglib:3.3.0' implementation 'commons-validator:commons-validator:1.7' implementation 'org.apache.commons:commons-lang3:3.12.0' implementation 'commons-io:commons-io:2.12.0' 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 8bb282859..b1119f34e 100644 --- a/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java +++ b/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java @@ -32,6 +32,7 @@ import org.openqa.selenium.support.pagefactory.ElementLocator; import org.openqa.selenium.support.pagefactory.FieldDecorator; +import javax.annotation.Nullable; import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.ParameterizedType; @@ -92,17 +93,17 @@ public AppiumFieldDecorator(SearchContext context, Duration duration) { this.duration = duration; defaultElementFieldDecoracor = new DefaultFieldDecorator( - new AppiumElementLocatorFactory(context, duration, - new DefaultElementByBuilder(platform, automation))) { + new AppiumElementLocatorFactory(context, duration, new DefaultElementByBuilder(platform, automation)) + ) { @Override protected WebElement proxyForLocator(ClassLoader ignored, ElementLocator locator) { return proxyForAnElement(locator); } @Override - @SuppressWarnings("unchecked") protected List proxyForListLocator(ClassLoader ignored, ElementLocator locator) { ElementListInterceptor elementInterceptor = new ElementListInterceptor(locator); + //noinspection unchecked return getEnhancedProxy(ArrayList.class, elementInterceptor); } @@ -121,14 +122,14 @@ protected boolean isDecoratableList(Field field) { 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(context, duration, new WidgetByBuilder(platform, automation)); + widgetLocatorFactory = new AppiumElementLocatorFactory( + context, duration, new WidgetByBuilder(platform, automation) + ); } public AppiumFieldDecorator(SearchContext context) { @@ -144,14 +145,10 @@ public AppiumFieldDecorator(SearchContext context) { */ public Object decorate(ClassLoader ignored, Field field) { Object result = defaultElementFieldDecoracor.decorate(ignored, field); - if (result != null) { - return result; - } - - return decorateWidget(field); + return result == null ? decorateWidget(field) : result; } - @SuppressWarnings("unchecked") + @Nullable private Object decorateWidget(Field field) { Class type = field.getType(); if (!Widget.class.isAssignableFrom(type) && !List.class.isAssignableFrom(type)) { @@ -177,30 +174,34 @@ private Object decorateWidget(Field field) { if (!Widget.class.isAssignableFrom((Class) listType)) { return null; } + //noinspection unchecked widgetType = (Class) listType; } else { return null; } } else { + //noinspection unchecked widgetType = (Class) field.getType(); } CacheableLocator locator = widgetLocatorFactory.createLocator(field); - Map> map = - OverrideWidgetReader.read(widgetType, field, platform); + Map> map = OverrideWidgetReader.read(widgetType, field, platform); if (isAlist) { - return getEnhancedProxy(ArrayList.class, - new WidgetListInterceptor(locator, webDriver, map, widgetType, - duration)); + return getEnhancedProxy( + ArrayList.class, + new WidgetListInterceptor(locator, webDriver, map, widgetType, duration) + ); } - Constructor constructor = - WidgetConstructorUtil.findConvenientConstructor(widgetType); - return getEnhancedProxy(widgetType, new Class[]{constructor.getParameterTypes()[0]}, + Constructor constructor = WidgetConstructorUtil.findConvenientConstructor(widgetType); + return getEnhancedProxy( + widgetType, + new Class[]{constructor.getParameterTypes()[0]}, new Object[]{proxyForAnElement(locator)}, - new WidgetInterceptor(locator, webDriver, null, map, duration)); + new WidgetInterceptor(locator, webDriver, null, map, duration) + ); } private WebElement proxyForAnElement(ElementLocator locator) { 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 efe7bd95f..5047fcc11 100644 --- a/src/main/java/io/appium/java_client/pagefactory/ElementInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/ElementInterceptor.java @@ -28,13 +28,14 @@ /** * Intercepts requests to {@link WebElement}. */ -class ElementInterceptor extends InterceptorOfASingleElement { +public class ElementInterceptor extends InterceptorOfASingleElement { - ElementInterceptor(ElementLocator locator, WebDriver driver) { + public ElementInterceptor(ElementLocator locator, WebDriver driver) { super(locator, driver); } - @Override protected Object getObject(WebElement element, Method method, Object[] args) + @Override + protected Object getObject(WebElement element, Method method, Object[] args) throws Throwable { try { return method.invoke(element, args); diff --git a/src/main/java/io/appium/java_client/pagefactory/ElementListInterceptor.java b/src/main/java/io/appium/java_client/pagefactory/ElementListInterceptor.java index fc7164533..adeec4455 100644 --- a/src/main/java/io/appium/java_client/pagefactory/ElementListInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/ElementListInterceptor.java @@ -28,13 +28,14 @@ /** * Intercepts requests to the list of {@link WebElement}. */ -class ElementListInterceptor extends InterceptorOfAListOfElements { +public class ElementListInterceptor extends InterceptorOfAListOfElements { - ElementListInterceptor(ElementLocator locator) { + public ElementListInterceptor(ElementLocator locator) { super(locator); } - @Override protected Object getObject(List elements, Method method, Object[] args) + @Override + protected Object getObject(List elements, Method method, Object[] args) throws Throwable { try { return method.invoke(elements, args); diff --git a/src/main/java/io/appium/java_client/pagefactory/ThrowableUtil.java b/src/main/java/io/appium/java_client/pagefactory/ThrowableUtil.java index ca65e9e24..c025a53dd 100644 --- a/src/main/java/io/appium/java_client/pagefactory/ThrowableUtil.java +++ b/src/main/java/io/appium/java_client/pagefactory/ThrowableUtil.java @@ -33,8 +33,8 @@ protected static boolean isInvalidSelectorRootCause(Throwable e) { return true; } - if (String.valueOf(e.getMessage()).contains(INVALID_SELECTOR_PATTERN) || String - .valueOf(e.getMessage()).contains("Locator Strategy \\w+ is not supported")) { + if (String.valueOf(e.getMessage()).contains(INVALID_SELECTOR_PATTERN) + || String.valueOf(e.getMessage()).contains("Locator Strategy \\w+ is not supported")) { return true; } @@ -54,8 +54,8 @@ protected static boolean isStaleElementReferenceException(Throwable e) { } protected static Throwable extractReadableException(Throwable e) { - if (!RuntimeException.class.equals(e.getClass()) && !InvocationTargetException.class - .equals(e.getClass())) { + if (!RuntimeException.class.equals(e.getClass()) + && !InvocationTargetException.class.equals(e.getClass())) { return e; } 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 2c3296d35..1155fc7cc 100644 --- a/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java @@ -19,17 +19,18 @@ import io.appium.java_client.pagefactory.bys.ContentType; import io.appium.java_client.pagefactory.interceptors.InterceptorOfASingleElement; import io.appium.java_client.pagefactory.locator.CacheableLocator; -import net.sf.cglib.proxy.MethodProxy; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; import org.openqa.selenium.support.PageFactory; +import javax.annotation.Nullable; import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.time.Duration; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.Callable; import static io.appium.java_client.pagefactory.ThrowableUtil.extractReadableException; import static io.appium.java_client.pagefactory.utils.WebDriverUnpackUtility.getCurrentContentType; @@ -41,33 +42,35 @@ class WidgetInterceptor extends InterceptorOfASingleElement { private final Duration duration; private WebElement cachedElement; - WidgetInterceptor(CacheableLocator locator, WebDriver driver, WebElement cachedElement, - Map> instantiationMap, - Duration duration) { + WidgetInterceptor( + CacheableLocator locator, + WebDriver driver, + @Nullable + WebElement cachedElement, + Map> instantiationMap, + Duration duration + ) { super(locator, driver); this.cachedElement = cachedElement; this.instantiationMap = instantiationMap; this.duration = duration; } - - @Override protected Object getObject(WebElement element, Method method, Object[] args) - throws Throwable { + @Override protected Object getObject(WebElement element, Method method, Object[] args) throws Throwable { ContentType type = getCurrentContentType(element); if (cachedElement == null - || (locator != null && !((CacheableLocator) locator) - .isLookUpCached()) - || cachedInstances.size() == 0) { + || (locator != null && !((CacheableLocator) locator).isLookUpCached()) + || cachedInstances.isEmpty() + ) { cachedElement = element; Constructor constructor = instantiationMap.get(type); Class clazz = constructor.getDeclaringClass(); - int modifiers = clazz.getModifiers(); - if (Modifier.isAbstract(modifiers)) { - throw new InstantiationException(clazz.getName() - + " is abstract so " - + "it can't be instantiated"); + if (Modifier.isAbstract(clazz.getModifiers())) { + throw new InstantiationException( + String.format("%s is abstract so it cannot be instantiated", clazz.getName()) + ); } Widget widget = constructor.newInstance(cachedElement); @@ -82,11 +85,10 @@ class WidgetInterceptor extends InterceptorOfASingleElement { } } - public Object intercept(Object obj, Method method, Object[] args, MethodProxy proxy) - throws Throwable { - if (locator != null) { - return super.intercept(obj, method, args, proxy); - } - return getObject(cachedElement, method, args); + @Override + public Object call(Object obj, Method method, Object[] args, Callable original) throws Throwable { + return locator == null + ? getObject(cachedElement, 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 04115aaa9..c48b7b3ac 100644 --- a/src/main/java/io/appium/java_client/pagefactory/WidgetListInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/WidgetListInterceptor.java @@ -19,7 +19,6 @@ import io.appium.java_client.pagefactory.bys.ContentType; import io.appium.java_client.pagefactory.interceptors.InterceptorOfAListOfElements; import io.appium.java_client.pagefactory.locator.CacheableLocator; -import io.appium.java_client.pagefactory.utils.ProxyFactory; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; @@ -31,11 +30,11 @@ import java.util.Map; import static io.appium.java_client.pagefactory.ThrowableUtil.extractReadableException; +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; -class WidgetListInterceptor extends InterceptorOfAListOfElements { - +public class WidgetListInterceptor extends InterceptorOfAListOfElements { private final Map> instantiationMap; private final List cachedWidgets = new ArrayList<>(); private final Class declaredType; @@ -43,7 +42,7 @@ class WidgetListInterceptor extends InterceptorOfAListOfElements { private final WebDriver driver; private List cachedElements; - WidgetListInterceptor(CacheableLocator locator, WebDriver driver, + public WidgetListInterceptor(CacheableLocator locator, WebDriver driver, Map> instantiationMap, Class declaredType, Duration duration) { super(locator); @@ -53,22 +52,22 @@ class WidgetListInterceptor extends InterceptorOfAListOfElements { this.driver = driver; } - - @Override protected Object getObject(List elements, Method method, Object[] args) - throws Throwable { - if (cachedElements == null || (locator != null && !((CacheableLocator) locator) - .isLookUpCached())) { + @Override + protected Object getObject(List elements, Method method, Object[] args) throws Throwable { + if (cachedElements == null || (locator != null && !((CacheableLocator) locator).isLookUpCached())) { cachedElements = elements; cachedWidgets.clear(); ContentType type = null; for (WebElement element : cachedElements) { type = ofNullable(type).orElseGet(() -> getCurrentContentType(element)); - Class[] params = - new Class[] {instantiationMap.get(type).getParameterTypes()[0]}; - cachedWidgets.add(ProxyFactory - .getEnhancedProxy(declaredType, params, new Object[] {element}, - new WidgetInterceptor(null, driver, element, instantiationMap, duration))); + Class[] params = new Class[] {instantiationMap.get(type).getParameterTypes()[0]}; + cachedWidgets.add( + getEnhancedProxy( + declaredType, params, new Object[] {element}, + new WidgetInterceptor(null, driver, element, instantiationMap, duration) + ) + ); } } try { 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 8b96c6c68..d276ce616 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 @@ -16,33 +16,30 @@ package io.appium.java_client.pagefactory.interceptors; -import net.sf.cglib.proxy.MethodInterceptor; -import net.sf.cglib.proxy.MethodProxy; +import io.appium.java_client.proxy.MethodCallListener; import org.openqa.selenium.WebElement; import org.openqa.selenium.support.pagefactory.ElementLocator; -import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.Callable; -public abstract class InterceptorOfAListOfElements implements MethodInterceptor { +public abstract class InterceptorOfAListOfElements implements MethodCallListener { protected final ElementLocator locator; public InterceptorOfAListOfElements(ElementLocator locator) { this.locator = locator; } - protected abstract Object getObject(List elements, Method method, Object[] args) - throws InvocationTargetException, IllegalAccessException, InstantiationException, Throwable; + protected abstract Object getObject( + List elements, Method method, Object[] args + ) throws Throwable; - /** - * Look at {@link MethodInterceptor#intercept(Object, Method, Object[], MethodProxy)}. - */ - public Object intercept(Object obj, Method method, Object[] args, MethodProxy proxy) - throws Throwable { + @Override + public Object call(Object obj, Method method, Object[] args, Callable original) throws Throwable { if (Object.class.equals(method.getDeclaringClass())) { - return proxy.invokeSuper(obj, args); + return original.call(); } List realElements = new ArrayList<>(locator.findElements()); 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 d06555eaf..c457353ae 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 @@ -16,16 +16,16 @@ package io.appium.java_client.pagefactory.interceptors; -import net.sf.cglib.proxy.MethodInterceptor; -import net.sf.cglib.proxy.MethodProxy; +import io.appium.java_client.proxy.MethodCallListener; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; import org.openqa.selenium.WrapsDriver; import org.openqa.selenium.support.pagefactory.ElementLocator; import java.lang.reflect.Method; +import java.util.concurrent.Callable; -public abstract class InterceptorOfASingleElement implements MethodInterceptor { +public abstract class InterceptorOfASingleElement implements MethodCallListener { protected final ElementLocator locator; protected final WebDriver driver; @@ -37,22 +37,18 @@ public InterceptorOfASingleElement(ElementLocator locator, WebDriver driver) { protected abstract Object getObject(WebElement element, Method method, Object[] args) throws Throwable; - /** - * Look at {@link MethodInterceptor#intercept(Object, Method, Object[], MethodProxy)}. - */ - public Object intercept(Object obj, Method method, Object[] args, MethodProxy proxy) - throws Throwable { - + @Override + public Object call(Object obj, Method method, Object[] args, Callable original) throws Throwable { if (method.getName().equals("toString") && args.length == 0) { return locator.toString(); } if (Object.class.equals(method.getDeclaringClass())) { - return proxy.invokeSuper(obj, args); + return original.call(); } - if (WrapsDriver.class.isAssignableFrom(method.getDeclaringClass()) && method.getName() - .equals("getWrappedDriver")) { + if (WrapsDriver.class.isAssignableFrom(method.getDeclaringClass()) + && method.getName().equals("getWrappedDriver")) { return driver; } 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 390ebcd92..18242a5fd 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,8 +16,21 @@ package io.appium.java_client.pagefactory.utils; -import net.sf.cglib.proxy.Enhancer; -import net.sf.cglib.proxy.MethodInterceptor; +import io.appium.java_client.proxy.MethodCallListener; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.matcher.ElementMatcher; +import net.bytebuddy.matcher.ElementMatchers; +import org.openqa.selenium.remote.RemoteWebElement; + +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; +import java.util.UUID; +import java.util.function.Consumer; + +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.namedOneOf; /** * Original class is a super class of a @@ -29,30 +42,55 @@ private ProxyFactory() { super(); } - public static T getEnhancedProxy(Class requiredClazz, MethodInterceptor interceptor) { - return getEnhancedProxy(requiredClazz, new Class[] {}, new Object[] {}, interceptor); + /** + * Creates a proxy instance for the given class with an empty constructor. + * + * @param The proxy object class. + * @param requiredClazz is a {@link java.lang.Class} whose instance should be created + * @param listener is the instance of a method listener class + * @return a proxied instance of the desired class + */ + public static T getEnhancedProxy(Class requiredClazz, MethodCallListener listener) { + return getEnhancedProxy(requiredClazz, new Class[] {}, new Object[] {}, listener); } /** - * It returns some proxies created by CGLIB. + * Creates a proxy instance for the given class. * * @param The proxy object class. - * @param requiredClazz is a {@link java.lang.Class} whose instance should be created + * @param cls is a {@link java.lang.Class} whose instance should be created * @param params is an array of @link java.lang.Class}. It should be convenient to * parameter types of some declared constructor which belongs to desired * class. * @param values is an array of @link java.lang.Object}. It should be convenient to * parameter types of some declared constructor which belongs to desired * class. - * @param interceptor is the instance of {@link net.sf.cglib.proxy.MethodInterceptor} + * @param listener is the instance of a method listener class * @return a proxied instance of the desired class */ - @SuppressWarnings("unchecked") - public static T getEnhancedProxy(Class requiredClazz, Class[] params, Object[] values, - MethodInterceptor interceptor) { - Enhancer enhancer = new Enhancer(); - enhancer.setSuperclass(requiredClazz); - enhancer.setCallback(interceptor); - return (T) enhancer.create(params, values); + public static T getEnhancedProxy( + Class cls, Class[] params, Object[] values, MethodCallListener listener + ) { + // This is an ugly hack that ensures a newly created instance of + // RemoteWebElement could be put into a map. + // By default, it cannot because + // @Override + // public int hashCode() { + // return id.hashCode(); + // } + // and, guess what the `id` property equals to for a newly created instance + Consumer onInstanceCreated = cls.isAssignableFrom(RemoteWebElement.class) + ? (instance) -> ((RemoteWebElement) instance).setId(UUID.randomUUID().toString()) + : null; + Set skippedMethods = new HashSet<>(OBJECT_METHOD_NAMES); + skippedMethods.add("iterator"); + skippedMethods.add("listIterator"); + skippedMethods.remove("toString"); + ElementMatcher extraMatcher = ElementMatchers.not(namedOneOf( + skippedMethods.toArray(new String[0]) + )); + return createProxy( + cls, values, params, Collections.singletonList(listener), onInstanceCreated, extraMatcher + ); } } diff --git a/src/main/java/io/appium/java_client/pagefactory/utils/WebDriverUnpackUtility.java b/src/main/java/io/appium/java_client/pagefactory/utils/WebDriverUnpackUtility.java index e6bb6f6d6..53ba1506c 100644 --- a/src/main/java/io/appium/java_client/pagefactory/utils/WebDriverUnpackUtility.java +++ b/src/main/java/io/appium/java_client/pagefactory/utils/WebDriverUnpackUtility.java @@ -24,6 +24,8 @@ import org.openqa.selenium.WrapsDriver; import org.openqa.selenium.WrapsElement; +import javax.annotation.Nullable; + import static io.appium.java_client.pagefactory.bys.ContentType.HTML_OR_DEFAULT; import static io.appium.java_client.pagefactory.bys.ContentType.NATIVE_MOBILE_SPECIFIC; import static java.util.Optional.ofNullable; @@ -45,21 +47,20 @@ public final class WebDriverUnpackUtility { * {@link WrapsDriver} or {@link WrapsElement} then this method returns null. * */ + @Nullable public static WebDriver unpackWebDriverFromSearchContext(SearchContext searchContext) { if (searchContext instanceof WebDriver) { return (WebDriver) searchContext; } if (searchContext instanceof WrapsDriver) { - return unpackWebDriverFromSearchContext( - ((WrapsDriver) searchContext).getWrappedDriver()); + return unpackWebDriverFromSearchContext(((WrapsDriver) searchContext).getWrappedDriver()); } // Search context it is not only WebDriver. WebElement is search context too. // RemoteWebElement implements WrapsDriver if (searchContext instanceof WrapsElement) { - return unpackWebDriverFromSearchContext( - ((WrapsElement) searchContext).getWrappedElement()); + return unpackWebDriverFromSearchContext(((WrapsElement) searchContext).getWrappedElement()); } return null; 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 286d7fc31..1722480ef 100644 --- a/src/main/java/io/appium/java_client/proxy/Helpers.java +++ b/src/main/java/io/appium/java_client/proxy/Helpers.java @@ -18,16 +18,30 @@ import com.google.common.base.Preconditions; import net.bytebuddy.ByteBuddy; +import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.dynamic.loading.ClassLoadingStrategy; import net.bytebuddy.implementation.MethodDelegation; +import net.bytebuddy.matcher.ElementMatcher; import net.bytebuddy.matcher.ElementMatchers; +import javax.annotation.Nullable; +import java.lang.reflect.Method; import java.util.Collection; import java.util.Collections; +import java.util.Set; +import java.util.function.Consumer; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static net.bytebuddy.matcher.ElementMatchers.namedOneOf; public class Helpers { - private Helpers() { - } + + public static final Set OBJECT_METHOD_NAMES = Stream.of(Object.class.getMethods()) + .map(Method::getName) + .collect(Collectors.toSet()); + + private Helpers() {} /** * Creates a transparent proxy instance for the given class. @@ -53,6 +67,42 @@ public static T createProxy( Object[] constructorArgs, Class[] constructorArgTypes, Collection listeners + ) { + ElementMatcher extraMatcher = ElementMatchers.not(namedOneOf( + OBJECT_METHOD_NAMES.toArray(new String[0]) + )); + return createProxy(cls, constructorArgs, constructorArgTypes, listeners, null, extraMatcher); + } + + /** + * Creates a transparent proxy instance for the given class. + * It is possible to provide one or more method execution listeners + * or replace particular method calls completely. Callbacks + * defined in these listeners are going to be called when any + * **public** method of the given class is invoked. Overridden callbacks + * are expected to be skipped if they throw + * {@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 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 + * represent types of constructorArgs. + * @param listeners One or more method invocation listeners. + * @param onInstanceCreated Optional proxy instance postprocessor + * @param extraMethodMatcher Optional additional method proxy conditions + * @param Any class derived from Object + * @return Proxy instance + */ + public static T createProxy( + Class cls, + Object[] constructorArgs, + Class[] constructorArgTypes, + Collection listeners, + @Nullable Consumer onInstanceCreated, + @Nullable ElementMatcher extraMethodMatcher ) { Preconditions.checkArgument(constructorArgs.length == constructorArgTypes.length, String.format( @@ -64,17 +114,18 @@ public static T createProxy( Preconditions.checkArgument(cls != null, "Class must not be null"); Preconditions.checkArgument(!cls.isInterface(), "Class must not be an interface"); + ElementMatcher.Junction matcher = ElementMatchers.isPublic(); + if (extraMethodMatcher != null) { + matcher = matcher.and(extraMethodMatcher); + } + //noinspection resource Class proxy = new ByteBuddy() .subclass(cls) - .method(ElementMatchers.isPublic() - .and(ElementMatchers.not( - ElementMatchers.isDeclaredBy(Object.class) - .or(ElementMatchers.isOverriddenFrom(Object.class)) - ))) + .method(matcher) .intercept(MethodDelegation.to(Interceptor.class)) .make() - .load(cls.getClassLoader(), ClassLoadingStrategy.Default.WRAPPER) + .load(ClassLoader.getSystemClassLoader(), ClassLoadingStrategy.Default.WRAPPER) .getLoaded() .asSubclass(cls); @@ -83,6 +134,9 @@ public static T createProxy( T instance = (T) proxy .getConstructor(constructorArgTypes) .newInstance(constructorArgs); + if (onInstanceCreated != null) { + onInstanceCreated.accept(instance); + } Interceptor.LISTENERS.put(instance, listeners); return instance; } catch (SecurityException | ReflectiveOperationException e) { diff --git a/src/main/java/io/appium/java_client/proxy/Interceptor.java b/src/main/java/io/appium/java_client/proxy/Interceptor.java index d343fc9c1..b27f50470 100644 --- a/src/main/java/io/appium/java_client/proxy/Interceptor.java +++ b/src/main/java/io/appium/java_client/proxy/Interceptor.java @@ -27,19 +27,13 @@ import java.lang.reflect.Method; import java.util.Collection; import java.util.Map; -import java.util.Set; import java.util.UUID; import java.util.WeakHashMap; import java.util.concurrent.Callable; -import java.util.stream.Collectors; -import java.util.stream.Stream; public class Interceptor { private static final Logger logger = LoggerFactory.getLogger(Interceptor.class); public static final Map> LISTENERS = new WeakHashMap<>(); - private static final Set OBJECT_METHOD_NAMES = Stream.of(Object.class.getMethods()) - .map(Method::getName) - .collect(Collectors.toSet()); /** * A magic method used to wrap public method calls in classes @@ -51,6 +45,7 @@ public class Interceptor { * @param callable The reference to the non-patched callable to avoid call recursion. * @return Either the original method result or the patched one. */ + @SuppressWarnings("unused") @RuntimeType public static Object intercept( @This Object self, @@ -58,11 +53,17 @@ public static Object intercept( @AllArguments Object[] args, @SuperCall Callable callable ) throws Throwable { - if (OBJECT_METHOD_NAMES.contains(method.getName())) { + boolean hasListeners = false; + try { + hasListeners = LISTENERS.containsKey(self); + } catch (RuntimeException e) { + // ignore + } + if (!hasListeners) { return callable.call(); } Collection listeners = LISTENERS.get(self); - if (listeners == null || listeners.isEmpty()) { + if (listeners.isEmpty()) { return callable.call(); } diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/WidgetTest.java b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/WidgetTest.java index 6e1f4ab2b..2f8e2d60d 100644 --- a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/WidgetTest.java +++ b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/WidgetTest.java @@ -37,7 +37,6 @@ protected WidgetTest(AbstractApp app, WebDriver driver) { protected static void checkThatLocatorsAreCreatedCorrectly(DefaultStubWidget single, List multiple, By rootLocator, By subLocator) { - assertThat(single.toString(), containsString(rootLocator.toString())); assertThat(multiple.stream().map(DefaultStubWidget::toString).collect(toList()), contains(containsString(rootLocator.toString()), diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/CombinedAppTest.java b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/CombinedAppTest.java index 2a453c2d9..c3ee905fb 100644 --- a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/CombinedAppTest.java +++ b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/CombinedAppTest.java @@ -6,7 +6,6 @@ import io.appium.java_client.pagefactory_tests.widget.tests.AbstractStubWebDriver; import io.appium.java_client.pagefactory_tests.widget.tests.DefaultStubWidget; import io.appium.java_client.pagefactory_tests.widget.tests.android.DefaultAndroidWidget; -import io.appium.java_client.pagefactory_tests.widget.tests.windows.DefaultWindowsWidget; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -35,7 +34,6 @@ public static Stream data() { arguments(new CombinedApp(), new AbstractStubWebDriver.StubAndroidDriver(), DefaultAndroidWidget.class), arguments(new CombinedApp(), new AbstractStubWebDriver.StubIOSXCUITDriver(), DefaultIosXCUITWidget.class), - arguments(new CombinedApp(), new AbstractStubWebDriver.StubWindowsDriver(), DefaultWindowsWidget.class), arguments(new CombinedApp(), new AbstractStubWebDriver.StubBrowserDriver(), DefaultFindByWidget.class), arguments(new CombinedApp(), new AbstractStubWebDriver.StubAndroidBrowserOrWebViewDriver(), DefaultFindByWidget.class), @@ -72,14 +70,12 @@ public static class CombinedApp implements AbstractApp { @OverrideWidget(html = DefaultFindByWidget.class, androidUIAutomator = DefaultAndroidWidget.class, - iOSXCUITAutomation = DefaultIosXCUITWidget.class, - windowsAutomation = DefaultWindowsWidget.class) + iOSXCUITAutomation = DefaultIosXCUITWidget.class) private DefaultStubWidget singleWidget; @OverrideWidget(html = DefaultFindByWidget.class, androidUIAutomator = DefaultAndroidWidget.class, - iOSXCUITAutomation = DefaultIosXCUITWidget.class, - windowsAutomation = DefaultWindowsWidget.class) + iOSXCUITAutomation = DefaultIosXCUITWidget.class) private List multipleWidget; @Override diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/CombinedWidgetTest.java b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/CombinedWidgetTest.java index d54e3618c..3c1c9145d 100644 --- a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/CombinedWidgetTest.java +++ b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/combined/CombinedWidgetTest.java @@ -6,7 +6,6 @@ import io.appium.java_client.pagefactory_tests.widget.tests.AbstractStubWebDriver; import io.appium.java_client.pagefactory_tests.widget.tests.DefaultStubWidget; import io.appium.java_client.pagefactory_tests.widget.tests.android.DefaultAndroidWidget; -import io.appium.java_client.pagefactory_tests.widget.tests.windows.DefaultWindowsWidget; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -38,8 +37,6 @@ public static Stream data() { new AbstractStubWebDriver.StubAndroidDriver(), DefaultAndroidWidget.class), Arguments.of(new AppWithCombinedWidgets(), new AbstractStubWebDriver.StubIOSXCUITDriver(), DefaultIosXCUITWidget.class), - Arguments.of(new AppWithCombinedWidgets(), - new AbstractStubWebDriver.StubWindowsDriver(), DefaultWindowsWidget.class), Arguments.of(new AppWithCombinedWidgets(), new AbstractStubWebDriver.StubBrowserDriver(), DefaultFindByWidget.class), Arguments.of(new AppWithCombinedWidgets(), @@ -79,15 +76,13 @@ public static class CombinedWidget extends DefaultStubWidget { @OverrideWidget(html = DefaultFindByWidget.class, androidUIAutomator = DefaultAndroidWidget.class, - iOSXCUITAutomation = DefaultIosXCUITWidget.class, - windowsAutomation = DefaultWindowsWidget.class + iOSXCUITAutomation = DefaultIosXCUITWidget.class ) private DefaultStubWidget singleWidget; @OverrideWidget(html = DefaultFindByWidget.class, androidUIAutomator = DefaultAndroidWidget.class, - iOSXCUITAutomation = DefaultIosXCUITWidget.class, - windowsAutomation = DefaultWindowsWidget.class + iOSXCUITAutomation = DefaultIosXCUITWidget.class ) private List multipleWidget; diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/windows/AnnotatedWindowsWidget.java b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/windows/AnnotatedWindowsWidget.java deleted file mode 100644 index 733d0db95..000000000 --- a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/windows/AnnotatedWindowsWidget.java +++ /dev/null @@ -1,15 +0,0 @@ -package io.appium.java_client.pagefactory_tests.widget.tests.windows; - -import io.appium.java_client.pagefactory.WindowsFindBy; -import io.appium.java_client.pagefactory.iOSXCUITFindBy; -import org.openqa.selenium.WebElement; - -@WindowsFindBy(windowsAutomation = "SOME_ROOT_LOCATOR") -@iOSXCUITFindBy(iOSNsPredicate = "XCUIT_SOME_ROOT_LOCATOR") -public class AnnotatedWindowsWidget extends DefaultWindowsWidget { - public static String WINDOWS_ROOT_WIDGET_LOCATOR = "SOME_ROOT_LOCATOR"; - - protected AnnotatedWindowsWidget(WebElement element) { - super(element); - } -} diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/windows/DefaultWindowsWidget.java b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/windows/DefaultWindowsWidget.java deleted file mode 100644 index ab7b81a41..000000000 --- a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/windows/DefaultWindowsWidget.java +++ /dev/null @@ -1,35 +0,0 @@ -package io.appium.java_client.pagefactory_tests.widget.tests.windows; - -import io.appium.java_client.pagefactory.WindowsFindBy; -import io.appium.java_client.pagefactory.iOSXCUITFindBy; -import io.appium.java_client.pagefactory_tests.widget.tests.DefaultStubWidget; -import org.openqa.selenium.WebElement; - -import java.util.List; - -public class DefaultWindowsWidget extends DefaultStubWidget { - - public static String WINDOWS_SUB_WIDGET_LOCATOR = "SOME_SUB_LOCATOR"; - - @WindowsFindBy(windowsAutomation = "SOME_SUB_LOCATOR") - @iOSXCUITFindBy(iOSNsPredicate = "XCUIT_SOME_SUB_LOCATOR") - private DefaultWindowsWidget singleWidget; - - @WindowsFindBy(windowsAutomation = "SOME_SUB_LOCATOR") - @iOSXCUITFindBy(iOSNsPredicate = "XCUIT_SOME_SUB_LOCATOR") - private List multipleWidgets; - - protected DefaultWindowsWidget(WebElement element) { - super(element); - } - - @Override - public DefaultWindowsWidget getSubWidget() { - return singleWidget; - } - - @Override - public List getSubWidgets() { - return multipleWidgets; - } -} diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/windows/ExtendedWindowsWidget.java b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/windows/ExtendedWindowsWidget.java deleted file mode 100644 index 14cc95f65..000000000 --- a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/windows/ExtendedWindowsWidget.java +++ /dev/null @@ -1,9 +0,0 @@ -package io.appium.java_client.pagefactory_tests.widget.tests.windows; - -import org.openqa.selenium.WebElement; - -public class ExtendedWindowsWidget extends AnnotatedWindowsWidget { - protected ExtendedWindowsWidget(WebElement element) { - super(element); - } -} diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/windows/WindowsApp.java b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/windows/WindowsApp.java deleted file mode 100644 index 07eb5784d..000000000 --- a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/windows/WindowsApp.java +++ /dev/null @@ -1,125 +0,0 @@ -package io.appium.java_client.pagefactory_tests.widget.tests.windows; - -import io.appium.java_client.AppiumBy; -import io.appium.java_client.pagefactory.WindowsFindBy; -import io.appium.java_client.pagefactory.iOSXCUITFindBy; -import io.appium.java_client.pagefactory_tests.widget.tests.ExtendedApp; - -import java.util.List; - -public class WindowsApp implements ExtendedApp { - - public static String WINDOWS_DEFAULT_WIDGET_LOCATOR = "SOME_WINDOWS_DEFAULT_LOCATOR"; - - public static String WINDOWS_EXTERNALLY_DEFINED_WIDGET_LOCATOR = "WINDOWS_EXTERNALLY_DEFINED_WIDGET_LOCATOR"; - - @WindowsFindBy(windowsAutomation = "SOME_WINDOWS_DEFAULT_LOCATOR") - @iOSXCUITFindBy(iOSNsPredicate = "SOME_XCUIT_DEFAULT_LOCATOR") - private DefaultWindowsWidget singleIosWidget; - - @WindowsFindBy(windowsAutomation = "SOME_WINDOWS_DEFAULT_LOCATOR") - @iOSXCUITFindBy(iOSNsPredicate = "SOME_XCUIT_DEFAULT_LOCATOR") - private List multipleIosWidgets; - - /** - * This class is annotated by {@link WindowsFindBy} and - * {@link io.appium.java_client.pagefactory.iOSXCUITFindBy}. - * This field was added to check that locator is created correctly according to current platform. - * It is expected that the root element and sub-elements are found using - * {@link AppiumBy#windowsAutomation(String)} - */ - private AnnotatedWindowsWidget singleAnnotatedIosWidget; - - /** - * This class is annotated by {@link WindowsFindBy} and - * {@link io.appium.java_client.pagefactory.iOSXCUITFindBy}. - * This field was added to check that locator is created correctly according to current platform. - * It is expected that the root element and sub-elements are found using - * {@link AppiumBy#windowsAutomation(String)}. - */ - private List multipleIosIosWidgets; - - /** - * This class is not annotated by {@link WindowsFindBy} and - * {@link io.appium.java_client.pagefactory.iOSXCUITFindBy}. - * But the superclass is annotated by these annotations. This field was added to check that locator is - * created correctly according to current platform. - * It is expected that the root element and sub-elements are found using - * {@link AppiumBy#windowsAutomation(String)}. - */ - private ExtendedWindowsWidget singleExtendedIosWidget; - - /** - * This class is not annotated by {@link WindowsFindBy} and - * {@link io.appium.java_client.pagefactory.iOSXCUITFindBy}. - * But the superclass is annotated by these annotations. This field was added to check that locator is - * created correctly according to current platform. - * It is expected that the root element and sub-elements are found using - * {@link AppiumBy#windowsAutomation(String)}. - */ - private List multipleExtendedIosWidgets; - - /** - * This class is not annotated by {@link WindowsFindBy} and - * {@link io.appium.java_client.pagefactory.iOSXCUITFindBy}. - * But the superclass is annotated by these annotations. This field was added to check that locator is - * created correctly according to current platform. - * It is expected that the root element and sub-elements are found using - * {@link AppiumBy#windowsAutomation(String)}. - */ - @WindowsFindBy(windowsAutomation = "WINDOWS_EXTERNALLY_DEFINED_WIDGET_LOCATOR") - @iOSXCUITFindBy(iOSNsPredicate = "SOME_XCUIT_EXTERNALLY_DEFINED_LOCATOR") - private ExtendedWindowsWidget singleOverriddenIosWidget; - - /** - * This class is not annotated by {@link WindowsFindBy} and - * {@link io.appium.java_client.pagefactory.iOSXCUITFindBy}. - * But the superclass is annotated by these annotations. This field was added to check that locator is - * created correctly according to current platform. - * It is expected that the root element and sub-elements are found using - * {@link AppiumBy#windowsAutomation(String)}. - */ - @WindowsFindBy(windowsAutomation = "WINDOWS_EXTERNALLY_DEFINED_WIDGET_LOCATOR") - @iOSXCUITFindBy(iOSNsPredicate = "SOME_XCUIT_EXTERNALLY_DEFINED_LOCATOR") - private List multipleOverriddenIosWidgets; - - @Override - public DefaultWindowsWidget getWidget() { - return singleIosWidget; - } - - @Override - public List getWidgets() { - return multipleIosWidgets; - } - - @Override - public DefaultWindowsWidget getAnnotatedWidget() { - return singleAnnotatedIosWidget; - } - - @Override - public List getAnnotatedWidgets() { - return multipleIosIosWidgets; - } - - @Override - public DefaultWindowsWidget getExtendedWidget() { - return singleExtendedIosWidget; - } - - @Override - public List getExtendedWidgets() { - return multipleExtendedIosWidgets; - } - - @Override - public DefaultWindowsWidget getExtendedWidgetWithOverriddenLocators() { - return singleOverriddenIosWidget; - } - - @Override - public List getExtendedWidgetsWithOverriddenLocators() { - return multipleOverriddenIosWidgets; - } -} diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/windows/WindowsWidgetTest.java b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/windows/WindowsWidgetTest.java deleted file mode 100644 index 023b24819..000000000 --- a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/windows/WindowsWidgetTest.java +++ /dev/null @@ -1,51 +0,0 @@ -package io.appium.java_client.pagefactory_tests.widget.tests.windows; - -import io.appium.java_client.pagefactory_tests.widget.tests.AbstractStubWebDriver; -import io.appium.java_client.pagefactory_tests.widget.tests.ExtendedApp; -import io.appium.java_client.pagefactory_tests.widget.tests.WidgetTest; -import org.junit.jupiter.api.Test; - -import static io.appium.java_client.MobileBy.windowsAutomation; -import static io.appium.java_client.pagefactory_tests.widget.tests.windows.AnnotatedWindowsWidget.WINDOWS_ROOT_WIDGET_LOCATOR; -import static io.appium.java_client.pagefactory_tests.widget.tests.windows.DefaultWindowsWidget.WINDOWS_SUB_WIDGET_LOCATOR; -import static io.appium.java_client.pagefactory_tests.widget.tests.windows.WindowsApp.WINDOWS_DEFAULT_WIDGET_LOCATOR; -import static io.appium.java_client.pagefactory_tests.widget.tests.windows.WindowsApp.WINDOWS_EXTERNALLY_DEFINED_WIDGET_LOCATOR; - -public class WindowsWidgetTest extends WidgetTest { - - public WindowsWidgetTest() { - super(new WindowsApp(), new AbstractStubWebDriver.StubWindowsDriver()); - } - - @Test - @Override - public void checkThatWidgetsAreCreatedCorrectly() { - checkThatLocatorsAreCreatedCorrectly(app.getWidget(), app.getWidgets(), - windowsAutomation(WINDOWS_DEFAULT_WIDGET_LOCATOR), windowsAutomation(WINDOWS_SUB_WIDGET_LOCATOR)); - } - - @Test - @Override - public void checkCaseWhenWidgetClassHasDeclaredLocatorAnnotation() { - checkThatLocatorsAreCreatedCorrectly(((ExtendedApp) app).getAnnotatedWidget(), - ((ExtendedApp) app).getAnnotatedWidgets(), - windowsAutomation(WINDOWS_ROOT_WIDGET_LOCATOR), windowsAutomation(WINDOWS_SUB_WIDGET_LOCATOR)); - } - - @Test - @Override - public void checkCaseWhenWidgetClassHasNoDeclaredAnnotationButItHasSuperclass() { - checkThatLocatorsAreCreatedCorrectly(((ExtendedApp) app).getExtendedWidget(), - ((ExtendedApp) app).getExtendedWidgets(), - windowsAutomation(WINDOWS_ROOT_WIDGET_LOCATOR), windowsAutomation(WINDOWS_SUB_WIDGET_LOCATOR)); - } - - @Test - @Override - public void checkCaseWhenBothWidgetFieldAndClassHaveDeclaredAnnotations() { - checkThatLocatorsAreCreatedCorrectly(((ExtendedApp) app).getExtendedWidgetWithOverriddenLocators(), - ((ExtendedApp) app).getExtendedWidgetsWithOverriddenLocators(), - windowsAutomation(WINDOWS_EXTERNALLY_DEFINED_WIDGET_LOCATOR), - windowsAutomation(WINDOWS_SUB_WIDGET_LOCATOR)); - } -} From 14187493fdb820ac529ea49a3764059f8ea39ecc Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Thu, 25 May 2023 12:23:39 +0200 Subject: [PATCH 2/7] Fix'em all --- .../AppiumElementLocatorFactory.java | 4 +- .../pagefactory/DefaultElementByBuilder.java | 6 +- .../pagefactory/ElementListInterceptor.java | 3 +- .../InterceptorOfASingleElement.java | 3 +- .../pagefactory/utils/ProxyFactory.java | 33 ++---- .../io/appium/java_client/proxy/Helpers.java | 24 ++--- .../appium/java_client/proxy/Interceptor.java | 102 +++++++++++++++--- 7 files changed, 113 insertions(+), 62 deletions(-) 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 170cffa38..0fc8e3d38 100644 --- a/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocatorFactory.java +++ b/src/main/java/io/appium/java_client/pagefactory/AppiumElementLocatorFactory.java @@ -53,7 +53,9 @@ public AppiumElementLocatorFactory(SearchContext searchContext, Duration duratio return this.createLocator((AnnotatedElement) field); } - @Override public @Nullable CacheableLocator createLocator(AnnotatedElement annotatedElement) { + @Override + @Nullable + public CacheableLocator createLocator(AnnotatedElement annotatedElement) { Duration customDuration; if (annotatedElement.isAnnotationPresent(WithTimeout.class)) { WithTimeout withTimeout = annotatedElement.getAnnotation(WithTimeout.class); diff --git a/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java b/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java index 512c978cc..23195e5ce 100644 --- a/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java +++ b/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java @@ -206,15 +206,13 @@ public By buildBy() { String idOrName = ((Field) annotatedElementContainer.getAnnotated()).getName(); if (defaultBy == null && mobileNativeBy == null) { - defaultBy = - new ByIdOrName(((Field) annotatedElementContainer.getAnnotated()).getName()); + defaultBy = new ByIdOrName(((Field) annotatedElementContainer.getAnnotated()).getName()); mobileNativeBy = new By.ById(idOrName); return returnMappedBy(defaultBy, mobileNativeBy); } if (defaultBy == null) { - defaultBy = - new ByIdOrName(((Field) annotatedElementContainer.getAnnotated()).getName()); + defaultBy = new ByIdOrName(((Field) annotatedElementContainer.getAnnotated()).getName()); return returnMappedBy(defaultBy, mobileNativeBy); } diff --git a/src/main/java/io/appium/java_client/pagefactory/ElementListInterceptor.java b/src/main/java/io/appium/java_client/pagefactory/ElementListInterceptor.java index adeec4455..77e68a329 100644 --- a/src/main/java/io/appium/java_client/pagefactory/ElementListInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/ElementListInterceptor.java @@ -35,8 +35,7 @@ public ElementListInterceptor(ElementLocator locator) { } @Override - protected Object getObject(List elements, Method method, Object[] args) - throws Throwable { + protected Object getObject(List elements, Method method, Object[] args) throws Throwable { try { return method.invoke(elements, args); } catch (Throwable 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 c457353ae..7ac12b3d6 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 @@ -34,8 +34,7 @@ public InterceptorOfASingleElement(ElementLocator locator, WebDriver driver) { this.driver = driver; } - protected abstract Object getObject(WebElement element, Method method, Object[] args) - throws Throwable; + 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 { 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 18242a5fd..4dd8eb777 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 @@ -20,13 +20,11 @@ import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.matcher.ElementMatcher; import net.bytebuddy.matcher.ElementMatchers; -import org.openqa.selenium.remote.RemoteWebElement; +import java.util.Arrays; import java.util.Collections; import java.util.HashSet; import java.util.Set; -import java.util.UUID; -import java.util.function.Consumer; import static io.appium.java_client.proxy.Helpers.OBJECT_METHOD_NAMES; import static io.appium.java_client.proxy.Helpers.createProxy; @@ -37,6 +35,14 @@ * proxy object here. */ public final class ProxyFactory { + private static final Set NON_PROXYABLE_METHODS = setWithout(OBJECT_METHOD_NAMES, "toString"); + + @SuppressWarnings("unchecked") + private static Set setWithout(@SuppressWarnings("SameParameterValue") Set source, T... items) { + Set result = new HashSet<>(source); + Arrays.asList(items).forEach(result::remove); + return Collections.unmodifiableSet(result); + } private ProxyFactory() { super(); @@ -71,26 +77,9 @@ public static T getEnhancedProxy(Class requiredClazz, MethodCallListener public static T getEnhancedProxy( Class cls, Class[] params, Object[] values, MethodCallListener listener ) { - // This is an ugly hack that ensures a newly created instance of - // RemoteWebElement could be put into a map. - // By default, it cannot because - // @Override - // public int hashCode() { - // return id.hashCode(); - // } - // and, guess what the `id` property equals to for a newly created instance - Consumer onInstanceCreated = cls.isAssignableFrom(RemoteWebElement.class) - ? (instance) -> ((RemoteWebElement) instance).setId(UUID.randomUUID().toString()) - : null; - Set skippedMethods = new HashSet<>(OBJECT_METHOD_NAMES); - skippedMethods.add("iterator"); - skippedMethods.add("listIterator"); - skippedMethods.remove("toString"); ElementMatcher extraMatcher = ElementMatchers.not(namedOneOf( - skippedMethods.toArray(new String[0]) + NON_PROXYABLE_METHODS.toArray(new String[0]) )); - return createProxy( - cls, values, params, Collections.singletonList(listener), onInstanceCreated, extraMatcher - ); + return createProxy(cls, values, params, Collections.singletonList(listener), extraMatcher); } } 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 1722480ef..d45f5a163 100644 --- a/src/main/java/io/appium/java_client/proxy/Helpers.java +++ b/src/main/java/io/appium/java_client/proxy/Helpers.java @@ -29,7 +29,6 @@ import java.util.Collection; import java.util.Collections; import java.util.Set; -import java.util.function.Consumer; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -71,7 +70,7 @@ public static T createProxy( ElementMatcher extraMatcher = ElementMatchers.not(namedOneOf( OBJECT_METHOD_NAMES.toArray(new String[0]) )); - return createProxy(cls, constructorArgs, constructorArgTypes, listeners, null, extraMatcher); + return createProxy(cls, constructorArgs, constructorArgTypes, listeners, extraMatcher); } /** @@ -91,7 +90,6 @@ public static T createProxy( * @param constructorArgTypes Array of constructor argument types. Must * represent types of constructorArgs. * @param listeners One or more method invocation listeners. - * @param onInstanceCreated Optional proxy instance postprocessor * @param extraMethodMatcher Optional additional method proxy conditions * @param Any class derived from Object * @return Proxy instance @@ -101,7 +99,6 @@ public static T createProxy( Object[] constructorArgs, Class[] constructorArgTypes, Collection listeners, - @Nullable Consumer onInstanceCreated, @Nullable ElementMatcher extraMethodMatcher ) { Preconditions.checkArgument(constructorArgs.length == constructorArgTypes.length, @@ -115,14 +112,10 @@ public static T createProxy( Preconditions.checkArgument(!cls.isInterface(), "Class must not be an interface"); ElementMatcher.Junction matcher = ElementMatchers.isPublic(); - if (extraMethodMatcher != null) { - matcher = matcher.and(extraMethodMatcher); - } - //noinspection resource Class proxy = new ByteBuddy() .subclass(cls) - .method(matcher) + .method(extraMethodMatcher == null ? matcher : matcher.and(extraMethodMatcher)) .intercept(MethodDelegation.to(Interceptor.class)) .make() .load(ClassLoader.getSystemClassLoader(), ClassLoadingStrategy.Default.WRAPPER) @@ -130,15 +123,10 @@ public static T createProxy( .asSubclass(cls); try { - //noinspection unchecked - T instance = (T) proxy - .getConstructor(constructorArgTypes) - .newInstance(constructorArgs); - if (onInstanceCreated != null) { - onInstanceCreated.accept(instance); - } - Interceptor.LISTENERS.put(instance, listeners); - return instance; + return Interceptor.setListeners( + cls.cast(proxy.getConstructor(constructorArgTypes).newInstance(constructorArgs)), + listeners + ); } catch (SecurityException | ReflectiveOperationException e) { throw new IllegalStateException(String.format("Unable to create a proxy of %s", cls.getName()), e); } diff --git a/src/main/java/io/appium/java_client/proxy/Interceptor.java b/src/main/java/io/appium/java_client/proxy/Interceptor.java index b27f50470..a44d43c89 100644 --- a/src/main/java/io/appium/java_client/proxy/Interceptor.java +++ b/src/main/java/io/appium/java_client/proxy/Interceptor.java @@ -16,6 +16,7 @@ package io.appium.java_client.proxy; +import javafx.util.Pair; import net.bytebuddy.implementation.bind.annotation.AllArguments; import net.bytebuddy.implementation.bind.annotation.Origin; import net.bytebuddy.implementation.bind.annotation.RuntimeType; @@ -24,16 +25,100 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.lang.ref.WeakReference; import java.lang.reflect.Method; +import java.util.ArrayList; import java.util.Collection; -import java.util.Map; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; import java.util.UUID; -import java.util.WeakHashMap; import java.util.concurrent.Callable; +import java.util.concurrent.Semaphore; public class Interceptor { private static final Logger logger = LoggerFactory.getLogger(Interceptor.class); - public static final Map> LISTENERS = new WeakHashMap<>(); + // Previously WeakHashMap has been used because of O(1) lookup performance, although + // we had to change it to a list, which has O(N). The reason for that is that + // maps implicitly call `hashCode` API on instances, which might not always + // work as expected for arbitrary proxies + private static final List, Collection>> LISTENERS = new ArrayList<>(); + private static final Semaphore LISTENERS_GUARD = new Semaphore(1); + + /** + * Assign listeners for the particular proxied instance. + * + * @param instance The proxied instance. + * @param listeners Collection of listeners. + * @return The same given instance. + */ + public static T setListeners(T instance, Collection listeners) { + try { + LISTENERS_GUARD.acquire(); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + try { + int i = 0; + boolean wasInstancePresent = false; + while (i < LISTENERS.size()) { + Pair, Collection> pair = LISTENERS.get(i); + Object key = pair.getKey().get(); + if (key == null) { + // The instance has been garbage-collected + LISTENERS.remove(i); + continue; + } + + if (key == instance) { + pair.getValue().clear(); + pair.getValue().addAll(listeners); + wasInstancePresent = true; + } + i++; + } + if (!wasInstancePresent) { + LISTENERS.add(new Pair<>(new WeakReference<>(instance), new HashSet<>(listeners))); + } + } finally { + LISTENERS_GUARD.release(); + } + return instance; + } + + /** + * Fetches listeners for the particular proxied instance. + * + * @param instance The proxied instance. + */ + public static Collection getListeners(Object instance) { + try { + LISTENERS_GUARD.acquire(); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + try { + int i = 0; + Collection result = Collections.emptySet(); + while (i < LISTENERS.size()) { + Pair, Collection> pair = LISTENERS.get(i); + Object key = pair.getKey().get(); + if (key == null) { + // The instance has been garbage-collected + LISTENERS.remove(i); + continue; + } + + if (key == instance) { + result = pair.getValue(); + } + i++; + } + return result; + } finally { + LISTENERS_GUARD.release(); + } + } /** * A magic method used to wrap public method calls in classes @@ -53,16 +138,7 @@ public static Object intercept( @AllArguments Object[] args, @SuperCall Callable callable ) throws Throwable { - boolean hasListeners = false; - try { - hasListeners = LISTENERS.containsKey(self); - } catch (RuntimeException e) { - // ignore - } - if (!hasListeners) { - return callable.call(); - } - Collection listeners = LISTENERS.get(self); + Collection listeners = getListeners(self); if (listeners.isEmpty()) { return callable.call(); } From 1bf93f89344ae7a44695fd6ca86214e87ba3654c Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Thu, 25 May 2023 12:27:54 +0200 Subject: [PATCH 3/7] Make the interceptor public --- .../appium/java_client/pagefactory/WidgetInterceptor.java | 7 ++++--- 1 file changed, 4 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 1155fc7cc..fd1d0f8e2 100644 --- a/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java @@ -35,14 +35,14 @@ import static io.appium.java_client.pagefactory.ThrowableUtil.extractReadableException; import static io.appium.java_client.pagefactory.utils.WebDriverUnpackUtility.getCurrentContentType; -class WidgetInterceptor extends InterceptorOfASingleElement { +public class WidgetInterceptor extends InterceptorOfASingleElement { private final Map> instantiationMap; private final Map cachedInstances = new HashMap<>(); private final Duration duration; private WebElement cachedElement; - WidgetInterceptor( + public WidgetInterceptor( CacheableLocator locator, WebDriver driver, @Nullable @@ -56,7 +56,8 @@ class WidgetInterceptor extends InterceptorOfASingleElement { this.duration = duration; } - @Override protected Object getObject(WebElement element, Method method, Object[] args) throws Throwable { + @Override + protected Object getObject(WebElement element, Method method, Object[] args) throws Throwable { ContentType type = getCurrentContentType(element); if (cachedElement == null || (locator != null && !((CacheableLocator) locator).isLookUpCached()) From 9d8b70fc01b63c7f5d63f287ab860f18ff85b59a Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Thu, 25 May 2023 13:38:18 +0200 Subject: [PATCH 4/7] Introduce pairs --- .../appium/java_client/proxy/Interceptor.java | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/main/java/io/appium/java_client/proxy/Interceptor.java b/src/main/java/io/appium/java_client/proxy/Interceptor.java index a44d43c89..41e8748be 100644 --- a/src/main/java/io/appium/java_client/proxy/Interceptor.java +++ b/src/main/java/io/appium/java_client/proxy/Interceptor.java @@ -16,7 +16,6 @@ package io.appium.java_client.proxy; -import javafx.util.Pair; import net.bytebuddy.implementation.bind.annotation.AllArguments; import net.bytebuddy.implementation.bind.annotation.Origin; import net.bytebuddy.implementation.bind.annotation.RuntimeType; @@ -45,6 +44,25 @@ public class Interceptor { private static final List, Collection>> LISTENERS = new ArrayList<>(); private static final Semaphore LISTENERS_GUARD = new Semaphore(1); + private static class Pair { + private final K key; + private final V value; + + public Pair(K key, V value) { + this.key = key; + this.value = value; + } + + public K getKey() { + return key; + } + + public V getValue() { + return value; + } + } + + /** * Assign listeners for the particular proxied instance. * From 735cbfeffe648e36da8f9a53168c766885e9e5d3 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Thu, 25 May 2023 13:44:45 +0200 Subject: [PATCH 5/7] Use linkedlist --- src/main/java/io/appium/java_client/proxy/Interceptor.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/appium/java_client/proxy/Interceptor.java b/src/main/java/io/appium/java_client/proxy/Interceptor.java index 41e8748be..d7bfd8799 100644 --- a/src/main/java/io/appium/java_client/proxy/Interceptor.java +++ b/src/main/java/io/appium/java_client/proxy/Interceptor.java @@ -26,10 +26,10 @@ import java.lang.ref.WeakReference; import java.lang.reflect.Method; -import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashSet; +import java.util.LinkedList; import java.util.List; import java.util.UUID; import java.util.concurrent.Callable; @@ -41,7 +41,7 @@ public class Interceptor { // we had to change it to a list, which has O(N). The reason for that is that // maps implicitly call `hashCode` API on instances, which might not always // work as expected for arbitrary proxies - private static final List, Collection>> LISTENERS = new ArrayList<>(); + private static final List, Collection>> LISTENERS = new LinkedList<>(); private static final Semaphore LISTENERS_GUARD = new Semaphore(1); private static class Pair { From eb0ee08112a0ed5bb0b6b21a6ea5c12d3ce50455 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Thu, 25 May 2023 14:57:43 +0200 Subject: [PATCH 6/7] Fix them all --- .../pagefactory/WidgetInterceptor.java | 3 + .../pagefactory/WidgetListInterceptor.java | 3 + .../pagefactory/utils/ProxyFactory.java | 8 +- .../io/appium/java_client/proxy/Helpers.java | 5 +- .../appium/java_client/proxy/Interceptor.java | 108 +------------- .../proxy/ProxyListenersContainer.java | 138 ++++++++++++++++++ .../java_client/proxy/ProxyHelpersTest.java | 1 - 7 files changed, 155 insertions(+), 111 deletions(-) create mode 100644 src/main/java/io/appium/java_client/proxy/ProxyListenersContainer.java 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 fd1d0f8e2..9a9d1133d 100644 --- a/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java @@ -42,6 +42,9 @@ public class WidgetInterceptor extends InterceptorOfASingleElement { private final Duration duration; private WebElement cachedElement; + /** + * Proxy interceptor class for widgets. + */ public WidgetInterceptor( CacheableLocator locator, WebDriver driver, 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 c48b7b3ac..136ce8e4d 100644 --- a/src/main/java/io/appium/java_client/pagefactory/WidgetListInterceptor.java +++ b/src/main/java/io/appium/java_client/pagefactory/WidgetListInterceptor.java @@ -42,6 +42,9 @@ public class WidgetListInterceptor extends InterceptorOfAListOfElements { private final WebDriver driver; private List cachedElements; + /** + * Proxy interceptor class for lists of widgets. + */ public WidgetListInterceptor(CacheableLocator locator, WebDriver driver, Map> instantiationMap, Class declaredType, Duration duration) { 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 4dd8eb777..d0b8e4832 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 @@ -80,6 +80,12 @@ public static T getEnhancedProxy( ElementMatcher extraMatcher = ElementMatchers.not(namedOneOf( NON_PROXYABLE_METHODS.toArray(new String[0]) )); - return createProxy(cls, values, params, Collections.singletonList(listener), extraMatcher); + return createProxy( + cls, + values, + params, + Collections.singletonList(listener), + extraMatcher + ); } } 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 d45f5a163..2dd5912ca 100644 --- a/src/main/java/io/appium/java_client/proxy/Helpers.java +++ b/src/main/java/io/appium/java_client/proxy/Helpers.java @@ -40,7 +40,8 @@ public class Helpers { .map(Method::getName) .collect(Collectors.toSet()); - private Helpers() {} + private Helpers() { + } /** * Creates a transparent proxy instance for the given class. @@ -123,7 +124,7 @@ public static T createProxy( .asSubclass(cls); try { - return Interceptor.setListeners( + return ProxyListenersContainer.getInstance().setListeners( cls.cast(proxy.getConstructor(constructorArgTypes).newInstance(constructorArgs)), listeners ); diff --git a/src/main/java/io/appium/java_client/proxy/Interceptor.java b/src/main/java/io/appium/java_client/proxy/Interceptor.java index d7bfd8799..dce2fd807 100644 --- a/src/main/java/io/appium/java_client/proxy/Interceptor.java +++ b/src/main/java/io/appium/java_client/proxy/Interceptor.java @@ -24,119 +24,13 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.lang.ref.WeakReference; import java.lang.reflect.Method; import java.util.Collection; -import java.util.Collections; -import java.util.HashSet; -import java.util.LinkedList; -import java.util.List; import java.util.UUID; import java.util.concurrent.Callable; -import java.util.concurrent.Semaphore; public class Interceptor { private static final Logger logger = LoggerFactory.getLogger(Interceptor.class); - // Previously WeakHashMap has been used because of O(1) lookup performance, although - // we had to change it to a list, which has O(N). The reason for that is that - // maps implicitly call `hashCode` API on instances, which might not always - // work as expected for arbitrary proxies - private static final List, Collection>> LISTENERS = new LinkedList<>(); - private static final Semaphore LISTENERS_GUARD = new Semaphore(1); - - private static class Pair { - private final K key; - private final V value; - - public Pair(K key, V value) { - this.key = key; - this.value = value; - } - - public K getKey() { - return key; - } - - public V getValue() { - return value; - } - } - - - /** - * Assign listeners for the particular proxied instance. - * - * @param instance The proxied instance. - * @param listeners Collection of listeners. - * @return The same given instance. - */ - public static T setListeners(T instance, Collection listeners) { - try { - LISTENERS_GUARD.acquire(); - } catch (InterruptedException e) { - throw new RuntimeException(e); - } - try { - int i = 0; - boolean wasInstancePresent = false; - while (i < LISTENERS.size()) { - Pair, Collection> pair = LISTENERS.get(i); - Object key = pair.getKey().get(); - if (key == null) { - // The instance has been garbage-collected - LISTENERS.remove(i); - continue; - } - - if (key == instance) { - pair.getValue().clear(); - pair.getValue().addAll(listeners); - wasInstancePresent = true; - } - i++; - } - if (!wasInstancePresent) { - LISTENERS.add(new Pair<>(new WeakReference<>(instance), new HashSet<>(listeners))); - } - } finally { - LISTENERS_GUARD.release(); - } - return instance; - } - - /** - * Fetches listeners for the particular proxied instance. - * - * @param instance The proxied instance. - */ - public static Collection getListeners(Object instance) { - try { - LISTENERS_GUARD.acquire(); - } catch (InterruptedException e) { - throw new RuntimeException(e); - } - try { - int i = 0; - Collection result = Collections.emptySet(); - while (i < LISTENERS.size()) { - Pair, Collection> pair = LISTENERS.get(i); - Object key = pair.getKey().get(); - if (key == null) { - // The instance has been garbage-collected - LISTENERS.remove(i); - continue; - } - - if (key == instance) { - result = pair.getValue(); - } - i++; - } - return result; - } finally { - LISTENERS_GUARD.release(); - } - } /** * A magic method used to wrap public method calls in classes @@ -156,7 +50,7 @@ public static Object intercept( @AllArguments Object[] args, @SuperCall Callable callable ) throws Throwable { - Collection listeners = getListeners(self); + Collection listeners = ProxyListenersContainer.getInstance().getListeners(self); if (listeners.isEmpty()) { return callable.call(); } diff --git a/src/main/java/io/appium/java_client/proxy/ProxyListenersContainer.java b/src/main/java/io/appium/java_client/proxy/ProxyListenersContainer.java new file mode 100644 index 000000000..64e4b1506 --- /dev/null +++ b/src/main/java/io/appium/java_client/proxy/ProxyListenersContainer.java @@ -0,0 +1,138 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.appium.java_client.proxy; + +import java.lang.ref.WeakReference; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.LinkedList; +import java.util.List; +import java.util.concurrent.Semaphore; + +class ProxyListenersContainer { + private static ProxyListenersContainer INSTANCE; + private final Semaphore LISTENERS_GUARD = new Semaphore(1); + // Previously WeakHashMap has been used because of O(1) lookup performance, although + // we had to change it to a list, which has O(N). The reason for that is that + // maps implicitly call `hashCode` API on instances, which might not always + // work as expected for arbitrary proxies + private final List, Collection>> LISTENERS = new LinkedList<>(); + + private static class Pair { + private final K key; + private final V value; + + public Pair(K key, V value) { + this.key = key; + this.value = value; + } + + public K getKey() { + return key; + } + + public V getValue() { + return value; + } + } + + private ProxyListenersContainer() { + } + + public static synchronized ProxyListenersContainer getInstance() { + if (INSTANCE == null) { + INSTANCE = new ProxyListenersContainer(); + } + return INSTANCE; + } + + /** + * Assign listeners for the particular proxied instance. + * + * @param proxyInstance The proxied instance. + * @param listeners Collection of listeners. + * @return The same given instance. + */ + public T setListeners(T proxyInstance, Collection listeners) { + try { + LISTENERS_GUARD.acquire(); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + try { + int i = 0; + boolean wasInstancePresent = false; + while (i < LISTENERS.size()) { + Pair, Collection> pair = LISTENERS.get(i); + Object key = pair.getKey().get(); + if (key == null) { + // The instance has been garbage-collected + LISTENERS.remove(i); + continue; + } + + if (key == proxyInstance) { + pair.getValue().clear(); + pair.getValue().addAll(listeners); + wasInstancePresent = true; + } + i++; + } + if (!wasInstancePresent) { + LISTENERS.add(new Pair<>(new WeakReference<>(proxyInstance), new HashSet<>(listeners))); + } + } finally { + LISTENERS_GUARD.release(); + } + return proxyInstance; + } + + /** + * Fetches listeners for the particular proxied instance. + * + * @param proxyInstance The proxied instance. + */ + public Collection getListeners(Object proxyInstance) { + try { + LISTENERS_GUARD.acquire(); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + try { + int i = 0; + Collection result = Collections.emptySet(); + while (i < LISTENERS.size()) { + Pair, Collection> pair = LISTENERS.get(i); + Object key = pair.getKey().get(); + if (key == null) { + // The instance has been garbage-collected + LISTENERS.remove(i); + continue; + } + + if (key == proxyInstance) { + result = pair.getValue(); + } + i++; + } + return result; + } finally { + LISTENERS_GUARD.release(); + } + } + +} diff --git a/src/test/java/io/appium/java_client/proxy/ProxyHelpersTest.java b/src/test/java/io/appium/java_client/proxy/ProxyHelpersTest.java index 218c56270..a8767629d 100644 --- a/src/test/java/io/appium/java_client/proxy/ProxyHelpersTest.java +++ b/src/test/java/io/appium/java_client/proxy/ProxyHelpersTest.java @@ -93,7 +93,6 @@ public Object onError(Object obj, Method method, Object[] args, Throwable e) { } }; RemoteWebDriver driver = createProxy(RemoteWebDriver.class, Collections.singletonList(listener)); - assertThrows( IllegalStateException.class, () -> driver.get("http://example.com/") From 26e0cb0114c2386fec33e90d56d9bd63367cdea0 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Thu, 25 May 2023 15:03:55 +0200 Subject: [PATCH 7/7] Checkstyle --- .../proxy/ProxyListenersContainer.java | 42 ++++++++++--------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/src/main/java/io/appium/java_client/proxy/ProxyListenersContainer.java b/src/main/java/io/appium/java_client/proxy/ProxyListenersContainer.java index 64e4b1506..4b314b02b 100644 --- a/src/main/java/io/appium/java_client/proxy/ProxyListenersContainer.java +++ b/src/main/java/io/appium/java_client/proxy/ProxyListenersContainer.java @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package io.appium.java_client.proxy; import java.lang.ref.WeakReference; @@ -25,12 +26,20 @@ class ProxyListenersContainer { private static ProxyListenersContainer INSTANCE; - private final Semaphore LISTENERS_GUARD = new Semaphore(1); + + public static synchronized ProxyListenersContainer getInstance() { + if (INSTANCE == null) { + INSTANCE = new ProxyListenersContainer(); + } + return INSTANCE; + } + + private final Semaphore listenersGuard = new Semaphore(1); // Previously WeakHashMap has been used because of O(1) lookup performance, although // we had to change it to a list, which has O(N). The reason for that is that // maps implicitly call `hashCode` API on instances, which might not always // work as expected for arbitrary proxies - private final List, Collection>> LISTENERS = new LinkedList<>(); + private final List, Collection>> listeners = new LinkedList<>(); private static class Pair { private final K key; @@ -53,13 +62,6 @@ public V getValue() { private ProxyListenersContainer() { } - public static synchronized ProxyListenersContainer getInstance() { - if (INSTANCE == null) { - INSTANCE = new ProxyListenersContainer(); - } - return INSTANCE; - } - /** * Assign listeners for the particular proxied instance. * @@ -69,19 +71,19 @@ public static synchronized ProxyListenersContainer getInstance() { */ public T setListeners(T proxyInstance, Collection listeners) { try { - LISTENERS_GUARD.acquire(); + listenersGuard.acquire(); } catch (InterruptedException e) { throw new RuntimeException(e); } try { int i = 0; boolean wasInstancePresent = false; - while (i < LISTENERS.size()) { - Pair, Collection> pair = LISTENERS.get(i); + while (i < this.listeners.size()) { + Pair, Collection> pair = this.listeners.get(i); Object key = pair.getKey().get(); if (key == null) { // The instance has been garbage-collected - LISTENERS.remove(i); + this.listeners.remove(i); continue; } @@ -93,10 +95,10 @@ public T setListeners(T proxyInstance, Collection listen i++; } if (!wasInstancePresent) { - LISTENERS.add(new Pair<>(new WeakReference<>(proxyInstance), new HashSet<>(listeners))); + this.listeners.add(new Pair<>(new WeakReference<>(proxyInstance), new HashSet<>(listeners))); } } finally { - LISTENERS_GUARD.release(); + listenersGuard.release(); } return proxyInstance; } @@ -108,19 +110,19 @@ public T setListeners(T proxyInstance, Collection listen */ public Collection getListeners(Object proxyInstance) { try { - LISTENERS_GUARD.acquire(); + listenersGuard.acquire(); } catch (InterruptedException e) { throw new RuntimeException(e); } try { int i = 0; Collection result = Collections.emptySet(); - while (i < LISTENERS.size()) { - Pair, Collection> pair = LISTENERS.get(i); + while (i < listeners.size()) { + Pair, Collection> pair = listeners.get(i); Object key = pair.getKey().get(); if (key == null) { // The instance has been garbage-collected - LISTENERS.remove(i); + listeners.remove(i); continue; } @@ -131,7 +133,7 @@ public Collection getListeners(Object proxyInstance) { } return result; } finally { - LISTENERS_GUARD.release(); + listenersGuard.release(); } }