From 71a9f3971f7b6cb767afd8a8d6624d5efba0bed3 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Wed, 2 Nov 2022 19:49:26 +0100 Subject: [PATCH 1/7] feat: Add alternative proxy implemntation --- build.gradle | 1 + docs/The-event_firing.md | 81 ++++++++- .../io/appium/java_client/AppiumDriver.java | 8 + .../java_client/android/AndroidDriver.java | 5 + .../io/appium/java_client/ios/IOSDriver.java | 8 + .../io/appium/java_client/proxy/Helpers.java | 156 ++++++++++++++++++ .../appium/java_client/proxy/Interceptor.java | 113 +++++++++++++ .../java_client/proxy/MethodCallListener.java | 84 ++++++++++ .../proxy/NotImplementedException.java | 19 +++ .../java_client/proxy/ProxyHelpersTest.java | 125 ++++++++++++++ 10 files changed, 599 insertions(+), 1 deletion(-) create mode 100644 src/main/java/io/appium/java_client/proxy/Helpers.java create mode 100644 src/main/java/io/appium/java_client/proxy/Interceptor.java create mode 100644 src/main/java/io/appium/java_client/proxy/MethodCallListener.java create mode 100644 src/main/java/io/appium/java_client/proxy/NotImplementedException.java create mode 100644 src/test/java/io/appium/java_client/proxy/ProxyHelpersTest.java diff --git a/build.gradle b/build.gradle index bf02a50a4..8918b815d 100644 --- a/build.gradle +++ b/build.gradle @@ -198,6 +198,7 @@ task unitTest( type: Test ) { testLogging.exceptionFormat = 'full' filter { includeTestsMatching 'io.appium.java_client.internal.*' + includeTestsMatching 'io.appium.java_client.proxy.*' } } diff --git a/docs/The-event_firing.md b/docs/The-event_firing.md index 7fa0a58d6..9794a11f3 100644 --- a/docs/The-event_firing.md +++ b/docs/The-event_firing.md @@ -3,7 +3,7 @@ since v8.0.0 # The purpose This feature allows end user to organize the event logging on the client side. -Also this feature may be useful in a binding with standard or custom reporting +Also, this feature may be useful in a binding with standard or custom reporting frameworks. The feature has been introduced first since Selenium API v4. # The API @@ -40,6 +40,7 @@ Listeners should implement WebDriverListener. It supports three types of events: To use this decorator you have to prepare a listener, create a decorator using this listener, decorate the original WebDriver instance with this decorator and use the new WebDriver instance created by the decorator instead of the original one: + ```java WebDriver original = new AndroidDriver(); // it is expected that MyListener class implements WebDriverListener @@ -66,6 +67,7 @@ decorated.get("http://example.com/"); WebElement header = decorated.findElement(By.tagName("h1")); // if an error happens during any of these calls the the onError event is fired ``` + The instance of WebDriver created by the decorator implements all the same interfaces as the original driver. A listener can subscribe to "specific" or "generic" events (or both). A "specific" event correspond to a single specific method, a "generic" event correspond to any @@ -74,3 +76,80 @@ implement a method with a name derived from the target method to be watched. The for "before"-events receive the parameters passed to the decorated method. The listener methods for "after"-events receive the parameters passed to the decorated method as well as the result returned by this method. + +## createProxy API (since Java Client 8.3.0) + +This API is unique to Appium Java Client and does not exist in Selenium. The reason for +its existence is the fact that the original event listeners API provided by Selenium is limited +because it can only use interface types for decorator objects. For example, the code below won't +work: + +```java +IOSDriver driver = new IOSDriver(new URL("http://doesnot.matter/"), new ImmutableCapabilities()) +{ + @Override + protected void startSession(Capabilities capabilities) + { + // Override in a sake of simplicity to avoid the actual session start + } +}; +WebDriverListener webDriverListener = new WebDriverListener() +{ +}; +IOSDriver decoratedDriver = (IOSDriver) new EventFiringDecorator(IOSDriver.class, webDriverListener).decorate( + driver); +``` + +The last line throws `ClassCastException` because `decoratedDriver` is of type `IOSDriver`, +which is a class rather than an interface. +See the issue [#1694](https://github.com/appium/java-client/issues/1694) for more +details. In order to workaround this limitation a special proxy implementation has been created, +which is capable of decorating class types: + +```java +import io.appium.java_client.proxy.MethodCallListener; +import io.appium.java_client.proxy.NotImplementedException; + +import static io.appium.java_client.proxy.Helpers.createProxy; + +// ... + +MethodCallListener listener = new MethodCallListener() { + @Override + public void beforeCall(Object target, Method method, Object[] args) { + if (!method.getName().equals("get")) { + throw new NotImplementedException(); + } + acc.append("beforeCall ").append(method.getName()).append("\n"); + } + + @Override + public void afterCall(Object target, Method method, Object[] args, Object result) { + if (!method.getName().equals("get")) { + throw new NotImplementedException(); + } + acc.append("afterCall ").append(method.getName()).append("\n"); + } +}; +IOSDriver decoratedDriver = createProxy( + IOSDriver.class, + new Objetc[] {new URL("http://localhost:4723/"), new XCUITestOptions()}, + new Class<>[] {URL.class, Capabilities.class}, + Collections.singletonList(listener) +); + +decoratedDriver.get("http://example.com/"); + +assertThat(acc.toString().trim()).isEqualTo( + String.join("\n", + "beforeCall get", + "afterCall get" + ) +); +``` + +This proxy is not tied to WebDriver descendants and could be used to any classes that have +**public** constructors. It also allows to intercept exceptions thrown by **public** class methods and/or +change/replace the original methods behavior. It is important to know that callbacks are **not** invoked +for methods derived from the standard `Object` class, like `toString` or `equals`. +Check [unit tests](../src/test/java/io/appium/java_client/proxy/ProxyHelpersTest.java) for more examples. diff --git a/src/main/java/io/appium/java_client/AppiumDriver.java b/src/main/java/io/appium/java_client/AppiumDriver.java index 450822a69..b911baff4 100644 --- a/src/main/java/io/appium/java_client/AppiumDriver.java +++ b/src/main/java/io/appium/java_client/AppiumDriver.java @@ -68,6 +68,14 @@ public class AppiumDriver extends RemoteWebDriver implements protected final RemoteLocationContext locationContext; private final ExecuteMethod executeMethod; + // Needed for tests + protected AppiumDriver() { + super(); + remoteAddress = null; + executeMethod = new AppiumExecutionMethod(this); + locationContext = new RemoteLocationContext(executeMethod); + } + /** * Creates a new instance based on command {@code executor} and {@code capabilities}. * diff --git a/src/main/java/io/appium/java_client/android/AndroidDriver.java b/src/main/java/io/appium/java_client/android/AndroidDriver.java index 92ed19370..7d169c6c4 100644 --- a/src/main/java/io/appium/java_client/android/AndroidDriver.java +++ b/src/main/java/io/appium/java_client/android/AndroidDriver.java @@ -96,6 +96,11 @@ public class AndroidDriver extends AppiumDriver implements private StringWebSocketClient logcatClient; + // Needed for tests + protected AndroidDriver() { + super(); + } + /** * Creates a new instance based on command {@code executor} and {@code capabilities}. * diff --git a/src/main/java/io/appium/java_client/ios/IOSDriver.java b/src/main/java/io/appium/java_client/ios/IOSDriver.java index 00098b14c..89aa7f411 100644 --- a/src/main/java/io/appium/java_client/ios/IOSDriver.java +++ b/src/main/java/io/appium/java_client/ios/IOSDriver.java @@ -23,18 +23,21 @@ import io.appium.java_client.AppiumClientConfig; import io.appium.java_client.AppiumDriver; +import io.appium.java_client.AppiumExecutionMethod; import io.appium.java_client.HasAppStrings; import io.appium.java_client.HasDeviceTime; import io.appium.java_client.HasOnScreenKeyboard; import io.appium.java_client.HidesKeyboard; import io.appium.java_client.HidesKeyboardWithKeyName; import io.appium.java_client.InteractsWithApps; +import io.appium.java_client.MobileCommand; import io.appium.java_client.PullsFiles; import io.appium.java_client.LocksDevice; import io.appium.java_client.PerformsTouchActions; import io.appium.java_client.PushesFiles; import io.appium.java_client.SupportsLegacyAppManagement; import io.appium.java_client.battery.HasBattery; +import io.appium.java_client.remote.AppiumCommandExecutor; import io.appium.java_client.remote.SupportsContextSwitching; import io.appium.java_client.remote.SupportsLocation; import io.appium.java_client.remote.SupportsRotation; @@ -85,6 +88,11 @@ public class IOSDriver extends AppiumDriver implements private StringWebSocketClient syslogClient; + // Needed for tests + protected IOSDriver() { + super(); + } + /** * Creates a new instance based on command {@code executor} and {@code capabilities}. * diff --git a/src/main/java/io/appium/java_client/proxy/Helpers.java b/src/main/java/io/appium/java_client/proxy/Helpers.java new file mode 100644 index 000000000..f62ccfdf8 --- /dev/null +++ b/src/main/java/io/appium/java_client/proxy/Helpers.java @@ -0,0 +1,156 @@ +/* + * 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 com.google.common.base.Preconditions; +import net.bytebuddy.ByteBuddy; +import net.bytebuddy.dynamic.loading.ClassLoadingStrategy; +import net.bytebuddy.implementation.MethodDelegation; +import net.bytebuddy.matcher.ElementMatchers; + +import java.util.Collection; +import java.util.Collections; + +public class Helpers { + private Helpers() {} + + /** + * 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}. + * + * @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. + * @return Proxy instance + * @param Any class derived from Object + */ + public static T createProxy( + Class cls, + Object[] constructorArgs, + Class[] constructorArgTypes, + Collection listeners + ) { + Preconditions.checkArgument(constructorArgs.length == constructorArgTypes.length, + String.format( + "Constructor arguments array length %d must be equal to the types array length %d", + constructorArgs.length, constructorArgTypes.length + ) + ); + Preconditions.checkArgument(!listeners.isEmpty(), "The collection of listeners must not be empty"); + Preconditions.checkArgument(cls != null, "Class must not be null"); + Preconditions.checkArgument(!cls.isInterface(), "Class must not be an interface"); + + //noinspection resource + Class proxy = new ByteBuddy() + .subclass(cls) + .method(ElementMatchers.isPublic() + .and(ElementMatchers.not( + ElementMatchers.isDeclaredBy(Object.class) + .or(ElementMatchers.isOverriddenFrom(Object.class)) + ))) + .intercept(MethodDelegation.to(Interceptor.class)) + .make() + .load(cls.getClassLoader(), ClassLoadingStrategy.Default.WRAPPER) + .getLoaded() + .asSubclass(cls); + + try { + //noinspection unchecked + T instance = (T) proxy + .getConstructor(constructorArgTypes) + .newInstance(constructorArgs); + Interceptor.LISTENERS.put(instance, listeners); + return instance; + } catch (SecurityException | ReflectiveOperationException e) { + throw new IllegalStateException(String.format("Unable to create a proxy of %s", cls.getName()), e); + } + } + + /** + * 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 NotImplementedException. + * + * @param cls the class to which the proxy should be created. + * Must not be an interface. Must expose a constructor + * without arguments. + * @param listeners One or more method invocation listeners. + * @return Proxy instance + * @param Any class derived from Object + */ + public static T createProxy(Class cls, Collection listeners) { + return createProxy(cls, new Object[]{}, new Class[] {}, listeners); + } + + /** + * 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 NotImplementedException. + * + * @param cls the class to which the proxy should be created. + * Must not be an interface. Must expose a constructor + * without arguments. + * @param listener Method invocation listener. + * @return Proxy instance + * @param Any class derived from Object + */ + public static T createProxy(Class cls, MethodCallListener listener) { + return createProxy(cls, new Object[]{}, new Class[] {}, Collections.singletonList(listener)); + } + + /** + * 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 NotImplementedException. + * + * @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 listener Method invocation listener. + * @return Proxy instance + * @param Any class derived from Object + */ + public static T createProxy( + Class cls, + Object[] constructorArgs, + Class[] constructorArgTypes, + MethodCallListener listener + ) { + return createProxy(cls, constructorArgs, constructorArgTypes, Collections.singletonList(listener)); + } +} diff --git a/src/main/java/io/appium/java_client/proxy/Interceptor.java b/src/main/java/io/appium/java_client/proxy/Interceptor.java new file mode 100644 index 000000000..fcee136bb --- /dev/null +++ b/src/main/java/io/appium/java_client/proxy/Interceptor.java @@ -0,0 +1,113 @@ +/* + * 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 net.bytebuddy.implementation.bind.annotation.AllArguments; +import net.bytebuddy.implementation.bind.annotation.Origin; +import net.bytebuddy.implementation.bind.annotation.RuntimeType; +import net.bytebuddy.implementation.bind.annotation.SuperCall; +import net.bytebuddy.implementation.bind.annotation.This; + +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.logging.Level; +import java.util.logging.Logger; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +public class Interceptor { + private static final Logger logger = Logger.getLogger(Interceptor.class.getName()); + 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()); + + @RuntimeType + public static Object intercept( + @This Object self, + @Origin Method method, + @AllArguments Object[] args, + @SuperCall Callable callable + ) throws Throwable { + if (OBJECT_METHOD_NAMES.contains(method.getName())) { + return callable.call(); + } + Collection listeners = LISTENERS.get(self); + if (listeners == null || listeners.isEmpty()) { + return callable.call(); + } + + listeners.forEach(listener -> { + try { + listener.beforeCall(self, method, args); + } catch (NotImplementedException e) { + // ignore + } catch (Exception e) { + logger.log(Level.SEVERE, "Got an unexpected error in beforeCall listener", e); + } + }); + + final UUID noResult = UUID.randomUUID(); + Object result = noResult; + for (MethodCallListener listener: listeners) { + try { + result = listener.call(self, method, args, callable); + break; + } catch (NotImplementedException e) { + // ignore + } catch (Exception e) { + try { + return listener.onError(self, method, args, e); + } catch (NotImplementedException e1) { + // ignore + } + throw e; + } + } + if (noResult.equals(result)) { + try { + result = callable.call(); + } catch (Exception e) { + for (MethodCallListener listener: listeners) { + try { + return listener.onError(self, method, args, e); + } catch (NotImplementedException e1) { + // ignore + } + } + throw e; + } + } + + final Object endResult = result == noResult ? null : result; + listeners.forEach(listener -> { + try { + listener.afterCall(self, method, args, endResult); + } catch (NotImplementedException e) { + // ignore + } catch (Exception e) { + logger.log(Level.SEVERE, "Got an unexpected error in afterCall listener", e); + } + }); + return endResult; + } +} diff --git a/src/main/java/io/appium/java_client/proxy/MethodCallListener.java b/src/main/java/io/appium/java_client/proxy/MethodCallListener.java new file mode 100644 index 000000000..a9fb8c633 --- /dev/null +++ b/src/main/java/io/appium/java_client/proxy/MethodCallListener.java @@ -0,0 +1,84 @@ +/* + * 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.reflect.Method; +import java.util.concurrent.Callable; + +public interface MethodCallListener { + + /** + * The callback to be invoked before any public method of the proxy is called. + * The implementation is not expected to throw any exceptions. If a + * runtime exception is thrown then it is going to be silently logged. + * + * @param obj The proxy instance + * @param method Method to be called + * @param args Array of method arguments + */ + default void beforeCall(Object obj, Method method, Object[] args) { + throw new NotImplementedException(); + } + + /** + * Override this callback in order to change/customize the behavior + * of a single or multiple methods. The original method result + * will be replaced with the result returned by this callback. + * Also, any exception thrown by it will replace original method(s) + * exception. + * + * @param obj The proxy instance + * @param method Method to be replaced + * @param args Array of method arguments + * @param original The reference to the original method in case it is necessary to + * instrument its result. + * @return It is expected that the type of the returned argument could be cast to + * the returned type of the original method. + */ + default Object call(Object obj, Method method, Object[] args, Callable original) throws Throwable { + throw new NotImplementedException(); + } + + /** + * The callback to be invoked after any public method of the proxy is called. + * The implementation is not expected to throw any exceptions. If a + * runtime exception is thrown then it is going to be silently logged. + * + * @param obj The proxy instance + * @param method Method to be called + * @param args Array of method arguments + */ + default void afterCall(Object obj, Method method, Object[] args, Object result) { + throw new NotImplementedException(); + } + + /** + * The callback to be invoked if a public method or its + * {@link #call(Object, Method, Object[], Callable) Call} replacement throws an exception. + * + * @param obj The proxy instance + * @param method Method to be called + * @param args Array of method arguments + * @param e Exception instance thrown by the original method invocation. + * @return You could either (re)throw the exception in this callback or + * overwrite the behavior and return a result from it. It is expected that the + * type of the returned argument could be cast to the returned type of the original method. + */ + default Object onError(Object obj, Method method, Object[] args, Throwable e) throws Throwable { + throw new NotImplementedException(); + } +} diff --git a/src/main/java/io/appium/java_client/proxy/NotImplementedException.java b/src/main/java/io/appium/java_client/proxy/NotImplementedException.java new file mode 100644 index 000000000..eda60296a --- /dev/null +++ b/src/main/java/io/appium/java_client/proxy/NotImplementedException.java @@ -0,0 +1,19 @@ +/* + * 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; + +public class NotImplementedException extends RuntimeException {} diff --git a/src/test/java/io/appium/java_client/proxy/ProxyHelpersTest.java b/src/test/java/io/appium/java_client/proxy/ProxyHelpersTest.java new file mode 100644 index 000000000..500e71e0f --- /dev/null +++ b/src/test/java/io/appium/java_client/proxy/ProxyHelpersTest.java @@ -0,0 +1,125 @@ +/* + * 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 io.appium.java_client.ios.IOSDriver; +import org.junit.jupiter.api.Test; +import org.openqa.selenium.Capabilities; +import org.openqa.selenium.ImmutableCapabilities; +import org.openqa.selenium.remote.RemoteWebDriver; +import org.openqa.selenium.remote.UnreachableBrowserException; + +import java.lang.reflect.Method; +import java.net.MalformedURLException; +import java.net.URL; +import java.util.Collections; +import java.util.concurrent.Callable; + +import static io.appium.java_client.proxy.Helpers.createProxy; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.Is.is; +import static org.hamcrest.core.IsEqual.equalTo; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class ProxyHelpersTest { + + @Test + void shouldFireBeforeAndAfterEvents() { + final StringBuilder acc = new StringBuilder(); + MethodCallListener listener = new MethodCallListener() { + @Override + public void beforeCall(Object target, Method method, Object[] args) { + acc.append("beforeCall ").append(method.getName()).append("\n"); + // should be ignored + throw new IllegalStateException(); + } + + @Override + public void afterCall(Object target, Method method, Object[] args, Object result) { + acc.append("afterCall ").append(method.getName()).append("\n"); + // should be ignored + throw new IllegalStateException(); + } + }; + RemoteWebDriver driver = createProxy(RemoteWebDriver.class, Collections.singletonList(listener)); + + assertThrows( + UnreachableBrowserException.class, + () -> driver.get("http://example.com/") + ); + + assertThat(acc.toString().trim(), is(equalTo( + String.join("\n", + "beforeCall get", + "beforeCall getSessionId", + "afterCall getSessionId", + "beforeCall getCapabilities", + "afterCall getCapabilities", + "beforeCall getCapabilities", + "afterCall getCapabilities") + ))); + } + + @Test + void shouldFireErrorEvents() { + MethodCallListener listener = new MethodCallListener() { + @Override + public Object onError(Object obj, Method method, Object[] args, Throwable e) { + throw new IllegalStateException(); + } + }; + RemoteWebDriver driver = createProxy(RemoteWebDriver.class, Collections.singletonList(listener)); + + assertThrows( + IllegalStateException.class, + () -> driver.get("http://example.com/") + ); + } + + @Test + void shouldFireCallEvents() { + final StringBuilder acc = new StringBuilder(); + MethodCallListener listener = new MethodCallListener() { + @Override + public Object call(Object obj, Method method, Object[] args, Callable original) { + acc.append("onCall ").append(method.getName()).append("\n"); + throw new IllegalStateException(); + } + + @Override + public Object onError(Object obj, Method method, Object[] args, Throwable e) throws Throwable { + acc.append("onError ").append(method.getName()).append("\n"); + throw e; + } + }; + IOSDriver driver = createProxy( + IOSDriver.class, + Collections.singletonList(listener) + ); + + assertThrows( + IllegalStateException.class, + () -> driver.get("http://example.com/") + ); + + assertThat(acc.toString().trim(), is(equalTo( + String.join("\n", + "onCall get", + "onError get") + ))); + } +} From af348812dac9515fd14e9f92e5696cfc33f17007 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Wed, 2 Nov 2022 19:54:55 +0100 Subject: [PATCH 2/7] Fix style --- .../java_client/proxy/MethodCallListener.java | 114 +++++++++--------- 1 file changed, 57 insertions(+), 57 deletions(-) diff --git a/src/main/java/io/appium/java_client/proxy/MethodCallListener.java b/src/main/java/io/appium/java_client/proxy/MethodCallListener.java index a9fb8c633..5d0c23dc8 100644 --- a/src/main/java/io/appium/java_client/proxy/MethodCallListener.java +++ b/src/main/java/io/appium/java_client/proxy/MethodCallListener.java @@ -21,64 +21,64 @@ public interface MethodCallListener { - /** - * The callback to be invoked before any public method of the proxy is called. - * The implementation is not expected to throw any exceptions. If a - * runtime exception is thrown then it is going to be silently logged. - * - * @param obj The proxy instance - * @param method Method to be called - * @param args Array of method arguments - */ - default void beforeCall(Object obj, Method method, Object[] args) { - throw new NotImplementedException(); - } + /** + * The callback to be invoked before any public method of the proxy is called. + * The implementation is not expected to throw any exceptions. If a + * runtime exception is thrown then it is going to be silently logged. + * + * @param obj The proxy instance + * @param method Method to be called + * @param args Array of method arguments + */ + default void beforeCall(Object obj, Method method, Object[] args) { + throw new NotImplementedException(); + } - /** - * Override this callback in order to change/customize the behavior - * of a single or multiple methods. The original method result - * will be replaced with the result returned by this callback. - * Also, any exception thrown by it will replace original method(s) - * exception. - * - * @param obj The proxy instance - * @param method Method to be replaced - * @param args Array of method arguments - * @param original The reference to the original method in case it is necessary to - * instrument its result. - * @return It is expected that the type of the returned argument could be cast to - * the returned type of the original method. - */ - default Object call(Object obj, Method method, Object[] args, Callable original) throws Throwable { - throw new NotImplementedException(); - } + /** + * Override this callback in order to change/customize the behavior + * of a single or multiple methods. The original method result + * will be replaced with the result returned by this callback. + * Also, any exception thrown by it will replace original method(s) + * exception. + * + * @param obj The proxy instance + * @param method Method to be replaced + * @param args Array of method arguments + * @param original The reference to the original method in case it is necessary to + * instrument its result. + * @return It is expected that the type of the returned argument could be cast to + * the returned type of the original method. + */ + default Object call(Object obj, Method method, Object[] args, Callable original) throws Throwable { + throw new NotImplementedException(); + } - /** - * The callback to be invoked after any public method of the proxy is called. - * The implementation is not expected to throw any exceptions. If a - * runtime exception is thrown then it is going to be silently logged. - * - * @param obj The proxy instance - * @param method Method to be called - * @param args Array of method arguments - */ - default void afterCall(Object obj, Method method, Object[] args, Object result) { - throw new NotImplementedException(); - } + /** + * The callback to be invoked after any public method of the proxy is called. + * The implementation is not expected to throw any exceptions. If a + * runtime exception is thrown then it is going to be silently logged. + * + * @param obj The proxy instance + * @param method Method to be called + * @param args Array of method arguments + */ + default void afterCall(Object obj, Method method, Object[] args, Object result) { + throw new NotImplementedException(); + } - /** - * The callback to be invoked if a public method or its - * {@link #call(Object, Method, Object[], Callable) Call} replacement throws an exception. - * - * @param obj The proxy instance - * @param method Method to be called - * @param args Array of method arguments - * @param e Exception instance thrown by the original method invocation. - * @return You could either (re)throw the exception in this callback or - * overwrite the behavior and return a result from it. It is expected that the - * type of the returned argument could be cast to the returned type of the original method. - */ - default Object onError(Object obj, Method method, Object[] args, Throwable e) throws Throwable { - throw new NotImplementedException(); - } + /** + * The callback to be invoked if a public method or its + * {@link #call(Object, Method, Object[], Callable) Call} replacement throws an exception. + * + * @param obj The proxy instance + * @param method Method to be called + * @param args Array of method arguments + * @param e Exception instance thrown by the original method invocation. + * @return You could either (re)throw the exception in this callback or + * overwrite the behavior and return a result from it. It is expected that the + * type of the returned argument could be cast to the returned type of the original method. + */ + default Object onError(Object obj, Method method, Object[] args, Throwable e) throws Throwable { + throw new NotImplementedException(); + } } From 39fe031d7984307eb7f2fe5b60d096109d48790b Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Wed, 2 Nov 2022 19:59:54 +0100 Subject: [PATCH 3/7] More style --- .../io/appium/java_client/proxy/Helpers.java | 241 +++++++++--------- .../appium/java_client/proxy/Interceptor.java | 138 +++++----- .../proxy/NotImplementedException.java | 4 +- .../java_client/proxy/ProxyHelpersTest.java | 156 ++++++------ 4 files changed, 268 insertions(+), 271 deletions(-) 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 f62ccfdf8..286d7fc31 100644 --- a/src/main/java/io/appium/java_client/proxy/Helpers.java +++ b/src/main/java/io/appium/java_client/proxy/Helpers.java @@ -26,131 +26,132 @@ import java.util.Collections; public class Helpers { - private Helpers() {} + private Helpers() { + } - /** - * 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}. - * - * @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. - * @return Proxy instance - * @param Any class derived from Object - */ - public static T createProxy( - Class cls, - Object[] constructorArgs, - Class[] constructorArgTypes, - Collection listeners - ) { - Preconditions.checkArgument(constructorArgs.length == constructorArgTypes.length, - String.format( - "Constructor arguments array length %d must be equal to the types array length %d", - constructorArgs.length, constructorArgTypes.length - ) - ); - Preconditions.checkArgument(!listeners.isEmpty(), "The collection of listeners must not be empty"); - Preconditions.checkArgument(cls != null, "Class must not be null"); - Preconditions.checkArgument(!cls.isInterface(), "Class must not be an interface"); + /** + * 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}. + * + * @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 Any class derived from Object + * @return Proxy instance + */ + public static T createProxy( + Class cls, + Object[] constructorArgs, + Class[] constructorArgTypes, + Collection listeners + ) { + Preconditions.checkArgument(constructorArgs.length == constructorArgTypes.length, + String.format( + "Constructor arguments array length %d must be equal to the types array length %d", + constructorArgs.length, constructorArgTypes.length + ) + ); + Preconditions.checkArgument(!listeners.isEmpty(), "The collection of listeners must not be empty"); + Preconditions.checkArgument(cls != null, "Class must not be null"); + Preconditions.checkArgument(!cls.isInterface(), "Class must not be an interface"); - //noinspection resource - Class proxy = new ByteBuddy() - .subclass(cls) - .method(ElementMatchers.isPublic() - .and(ElementMatchers.not( - ElementMatchers.isDeclaredBy(Object.class) - .or(ElementMatchers.isOverriddenFrom(Object.class)) - ))) - .intercept(MethodDelegation.to(Interceptor.class)) - .make() - .load(cls.getClassLoader(), ClassLoadingStrategy.Default.WRAPPER) - .getLoaded() - .asSubclass(cls); + //noinspection resource + Class proxy = new ByteBuddy() + .subclass(cls) + .method(ElementMatchers.isPublic() + .and(ElementMatchers.not( + ElementMatchers.isDeclaredBy(Object.class) + .or(ElementMatchers.isOverriddenFrom(Object.class)) + ))) + .intercept(MethodDelegation.to(Interceptor.class)) + .make() + .load(cls.getClassLoader(), ClassLoadingStrategy.Default.WRAPPER) + .getLoaded() + .asSubclass(cls); - try { - //noinspection unchecked - T instance = (T) proxy - .getConstructor(constructorArgTypes) - .newInstance(constructorArgs); - Interceptor.LISTENERS.put(instance, listeners); - return instance; - } catch (SecurityException | ReflectiveOperationException e) { - throw new IllegalStateException(String.format("Unable to create a proxy of %s", cls.getName()), e); + try { + //noinspection unchecked + T instance = (T) proxy + .getConstructor(constructorArgTypes) + .newInstance(constructorArgs); + Interceptor.LISTENERS.put(instance, listeners); + return instance; + } catch (SecurityException | ReflectiveOperationException e) { + throw new IllegalStateException(String.format("Unable to create a proxy of %s", cls.getName()), e); + } } - } - /** - * 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 NotImplementedException. - * - * @param cls the class to which the proxy should be created. - * Must not be an interface. Must expose a constructor - * without arguments. - * @param listeners One or more method invocation listeners. - * @return Proxy instance - * @param Any class derived from Object - */ - public static T createProxy(Class cls, Collection listeners) { - return createProxy(cls, new Object[]{}, new Class[] {}, listeners); - } + /** + * 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 NotImplementedException. + * + * @param cls the class to which the proxy should be created. + * Must not be an interface. Must expose a constructor + * without arguments. + * @param listeners One or more method invocation listeners. + * @param Any class derived from Object + * @return Proxy instance + */ + public static T createProxy(Class cls, Collection listeners) { + return createProxy(cls, new Object[]{}, new Class[]{}, listeners); + } - /** - * 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 NotImplementedException. - * - * @param cls the class to which the proxy should be created. - * Must not be an interface. Must expose a constructor - * without arguments. - * @param listener Method invocation listener. - * @return Proxy instance - * @param Any class derived from Object - */ - public static T createProxy(Class cls, MethodCallListener listener) { - return createProxy(cls, new Object[]{}, new Class[] {}, Collections.singletonList(listener)); - } + /** + * 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 NotImplementedException. + * + * @param cls the class to which the proxy should be created. + * Must not be an interface. Must expose a constructor + * without arguments. + * @param listener Method invocation listener. + * @param Any class derived from Object + * @return Proxy instance + */ + public static T createProxy(Class cls, MethodCallListener listener) { + return createProxy(cls, new Object[]{}, new Class[]{}, Collections.singletonList(listener)); + } - /** - * 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 NotImplementedException. - * - * @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 listener Method invocation listener. - * @return Proxy instance - * @param Any class derived from Object - */ - public static T createProxy( - Class cls, - Object[] constructorArgs, - Class[] constructorArgTypes, - MethodCallListener listener - ) { - return createProxy(cls, constructorArgs, constructorArgTypes, Collections.singletonList(listener)); - } + /** + * 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 NotImplementedException. + * + * @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 listener Method invocation listener. + * @param Any class derived from Object + * @return Proxy instance + */ + public static T createProxy( + Class cls, + Object[] constructorArgs, + Class[] constructorArgTypes, + MethodCallListener listener + ) { + return createProxy(cls, constructorArgs, constructorArgTypes, Collections.singletonList(listener)); + } } 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 fcee136bb..cbb076bad 100644 --- a/src/main/java/io/appium/java_client/proxy/Interceptor.java +++ b/src/main/java/io/appium/java_client/proxy/Interceptor.java @@ -35,79 +35,79 @@ import java.util.stream.Stream; public class Interceptor { - private static final Logger logger = Logger.getLogger(Interceptor.class.getName()); - 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()); + private static final Logger logger = Logger.getLogger(Interceptor.class.getName()); + 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()); - @RuntimeType - public static Object intercept( - @This Object self, - @Origin Method method, - @AllArguments Object[] args, - @SuperCall Callable callable - ) throws Throwable { - if (OBJECT_METHOD_NAMES.contains(method.getName())) { - return callable.call(); - } - Collection listeners = LISTENERS.get(self); - if (listeners == null || listeners.isEmpty()) { - return callable.call(); - } + @RuntimeType + public static Object intercept( + @This Object self, + @Origin Method method, + @AllArguments Object[] args, + @SuperCall Callable callable + ) throws Throwable { + if (OBJECT_METHOD_NAMES.contains(method.getName())) { + return callable.call(); + } + Collection listeners = LISTENERS.get(self); + if (listeners == null || listeners.isEmpty()) { + return callable.call(); + } - listeners.forEach(listener -> { - try { - listener.beforeCall(self, method, args); - } catch (NotImplementedException e) { - // ignore - } catch (Exception e) { - logger.log(Level.SEVERE, "Got an unexpected error in beforeCall listener", e); - } - }); + listeners.forEach(listener -> { + try { + listener.beforeCall(self, method, args); + } catch (NotImplementedException e) { + // ignore + } catch (Exception e) { + logger.log(Level.SEVERE, "Got an unexpected error in beforeCall listener", e); + } + }); - final UUID noResult = UUID.randomUUID(); - Object result = noResult; - for (MethodCallListener listener: listeners) { - try { - result = listener.call(self, method, args, callable); - break; - } catch (NotImplementedException e) { - // ignore - } catch (Exception e) { - try { - return listener.onError(self, method, args, e); - } catch (NotImplementedException e1) { - // ignore + final UUID noResult = UUID.randomUUID(); + Object result = noResult; + for (MethodCallListener listener : listeners) { + try { + result = listener.call(self, method, args, callable); + break; + } catch (NotImplementedException e) { + // ignore + } catch (Exception e) { + try { + return listener.onError(self, method, args, e); + } catch (NotImplementedException e1) { + // ignore + } + throw e; + } } - throw e; - } - } - if (noResult.equals(result)) { - try { - result = callable.call(); - } catch (Exception e) { - for (MethodCallListener listener: listeners) { - try { - return listener.onError(self, method, args, e); - } catch (NotImplementedException e1) { - // ignore - } + if (noResult.equals(result)) { + try { + result = callable.call(); + } catch (Exception e) { + for (MethodCallListener listener : listeners) { + try { + return listener.onError(self, method, args, e); + } catch (NotImplementedException e1) { + // ignore + } + } + throw e; + } } - throw e; - } - } - final Object endResult = result == noResult ? null : result; - listeners.forEach(listener -> { - try { - listener.afterCall(self, method, args, endResult); - } catch (NotImplementedException e) { - // ignore - } catch (Exception e) { - logger.log(Level.SEVERE, "Got an unexpected error in afterCall listener", e); - } - }); - return endResult; - } + final Object endResult = result == noResult ? null : result; + listeners.forEach(listener -> { + try { + listener.afterCall(self, method, args, endResult); + } catch (NotImplementedException e) { + // ignore + } catch (Exception e) { + logger.log(Level.SEVERE, "Got an unexpected error in afterCall listener", e); + } + }); + return endResult; + } } diff --git a/src/main/java/io/appium/java_client/proxy/NotImplementedException.java b/src/main/java/io/appium/java_client/proxy/NotImplementedException.java index eda60296a..9c0425d8e 100644 --- a/src/main/java/io/appium/java_client/proxy/NotImplementedException.java +++ b/src/main/java/io/appium/java_client/proxy/NotImplementedException.java @@ -15,5 +15,5 @@ */ package io.appium.java_client.proxy; - -public class NotImplementedException extends RuntimeException {} +public class NotImplementedException extends RuntimeException { +} 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 500e71e0f..0e79538cf 100644 --- a/src/test/java/io/appium/java_client/proxy/ProxyHelpersTest.java +++ b/src/test/java/io/appium/java_client/proxy/ProxyHelpersTest.java @@ -18,14 +18,10 @@ import io.appium.java_client.ios.IOSDriver; import org.junit.jupiter.api.Test; -import org.openqa.selenium.Capabilities; -import org.openqa.selenium.ImmutableCapabilities; import org.openqa.selenium.remote.RemoteWebDriver; import org.openqa.selenium.remote.UnreachableBrowserException; import java.lang.reflect.Method; -import java.net.MalformedURLException; -import java.net.URL; import java.util.Collections; import java.util.concurrent.Callable; @@ -37,89 +33,89 @@ class ProxyHelpersTest { - @Test - void shouldFireBeforeAndAfterEvents() { - final StringBuilder acc = new StringBuilder(); - MethodCallListener listener = new MethodCallListener() { - @Override - public void beforeCall(Object target, Method method, Object[] args) { - acc.append("beforeCall ").append(method.getName()).append("\n"); - // should be ignored - throw new IllegalStateException(); - } + @Test + void shouldFireBeforeAndAfterEvents() { + final StringBuilder acc = new StringBuilder(); + MethodCallListener listener = new MethodCallListener() { + @Override + public void beforeCall(Object target, Method method, Object[] args) { + acc.append("beforeCall ").append(method.getName()).append("\n"); + // should be ignored + throw new IllegalStateException(); + } - @Override - public void afterCall(Object target, Method method, Object[] args, Object result) { - acc.append("afterCall ").append(method.getName()).append("\n"); - // should be ignored - throw new IllegalStateException(); - } - }; - RemoteWebDriver driver = createProxy(RemoteWebDriver.class, Collections.singletonList(listener)); + @Override + public void afterCall(Object target, Method method, Object[] args, Object result) { + acc.append("afterCall ").append(method.getName()).append("\n"); + // should be ignored + throw new IllegalStateException(); + } + }; + RemoteWebDriver driver = createProxy(RemoteWebDriver.class, Collections.singletonList(listener)); - assertThrows( - UnreachableBrowserException.class, - () -> driver.get("http://example.com/") - ); + assertThrows( + UnreachableBrowserException.class, + () -> driver.get("http://example.com/") + ); - assertThat(acc.toString().trim(), is(equalTo( - String.join("\n", - "beforeCall get", - "beforeCall getSessionId", - "afterCall getSessionId", - "beforeCall getCapabilities", - "afterCall getCapabilities", - "beforeCall getCapabilities", - "afterCall getCapabilities") - ))); - } + assertThat(acc.toString().trim(), is(equalTo( + String.join("\n", + "beforeCall get", + "beforeCall getSessionId", + "afterCall getSessionId", + "beforeCall getCapabilities", + "afterCall getCapabilities", + "beforeCall getCapabilities", + "afterCall getCapabilities") + ))); + } - @Test - void shouldFireErrorEvents() { - MethodCallListener listener = new MethodCallListener() { - @Override - public Object onError(Object obj, Method method, Object[] args, Throwable e) { - throw new IllegalStateException(); - } - }; - RemoteWebDriver driver = createProxy(RemoteWebDriver.class, Collections.singletonList(listener)); + @Test + void shouldFireErrorEvents() { + MethodCallListener listener = new MethodCallListener() { + @Override + public Object onError(Object obj, Method method, Object[] args, Throwable e) { + throw new IllegalStateException(); + } + }; + RemoteWebDriver driver = createProxy(RemoteWebDriver.class, Collections.singletonList(listener)); - assertThrows( - IllegalStateException.class, - () -> driver.get("http://example.com/") - ); - } + assertThrows( + IllegalStateException.class, + () -> driver.get("http://example.com/") + ); + } - @Test - void shouldFireCallEvents() { - final StringBuilder acc = new StringBuilder(); - MethodCallListener listener = new MethodCallListener() { - @Override - public Object call(Object obj, Method method, Object[] args, Callable original) { - acc.append("onCall ").append(method.getName()).append("\n"); - throw new IllegalStateException(); - } + @Test + void shouldFireCallEvents() { + final StringBuilder acc = new StringBuilder(); + MethodCallListener listener = new MethodCallListener() { + @Override + public Object call(Object obj, Method method, Object[] args, Callable original) { + acc.append("onCall ").append(method.getName()).append("\n"); + throw new IllegalStateException(); + } - @Override - public Object onError(Object obj, Method method, Object[] args, Throwable e) throws Throwable { - acc.append("onError ").append(method.getName()).append("\n"); - throw e; - } - }; - IOSDriver driver = createProxy( - IOSDriver.class, - Collections.singletonList(listener) - ); + @Override + public Object onError(Object obj, Method method, Object[] args, Throwable e) throws Throwable { + acc.append("onError ").append(method.getName()).append("\n"); + throw e; + } + }; + IOSDriver driver = createProxy( + IOSDriver.class, + Collections.singletonList(listener) + ); - assertThrows( - IllegalStateException.class, - () -> driver.get("http://example.com/") - ); + assertThrows( + IllegalStateException.class, + () -> driver.get("http://example.com/") + ); - assertThat(acc.toString().trim(), is(equalTo( - String.join("\n", - "onCall get", - "onError get") - ))); - } + assertThat(acc.toString().trim(), is(equalTo( + String.join("\n", + "onCall get", + "onError get") + ))); + } } From f8e315f21f1f0ccca6a9503589bba0f508841c8a Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Wed, 2 Nov 2022 20:11:43 +0100 Subject: [PATCH 4/7] Styling --- .../io/appium/java_client/proxy/Interceptor.java | 14 ++++++++++++-- .../java_client/proxy/MethodCallListener.java | 7 +++---- .../java_client/proxy/NotImplementedException.java | 1 + 3 files changed, 16 insertions(+), 6 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 cbb076bad..1601f1900 100644 --- a/src/main/java/io/appium/java_client/proxy/Interceptor.java +++ b/src/main/java/io/appium/java_client/proxy/Interceptor.java @@ -41,6 +41,16 @@ public class Interceptor { .map(Method::getName) .collect(Collectors.toSet()); + /** + * A magic method used to wrap public method calls in classes + * patched by ByteBuddy and acting as proxies. + * + * @param self The reference to the original instance. + * @param method The reference to the original method. + * @param args The reference to method args. + * @param callable The reference to the non-patched callable to avoid call recursion. + * @return Either the original method result or the patched one. + */ @RuntimeType public static Object intercept( @This Object self, @@ -77,7 +87,7 @@ public static Object intercept( } catch (Exception e) { try { return listener.onError(self, method, args, e); - } catch (NotImplementedException e1) { + } catch (NotImplementedException ignore) { // ignore } throw e; @@ -90,7 +100,7 @@ public static Object intercept( for (MethodCallListener listener : listeners) { try { return listener.onError(self, method, args, e); - } catch (NotImplementedException e1) { + } catch (NotImplementedException ignore) { // ignore } } diff --git a/src/main/java/io/appium/java_client/proxy/MethodCallListener.java b/src/main/java/io/appium/java_client/proxy/MethodCallListener.java index 5d0c23dc8..b35bb3dfd 100644 --- a/src/main/java/io/appium/java_client/proxy/MethodCallListener.java +++ b/src/main/java/io/appium/java_client/proxy/MethodCallListener.java @@ -44,8 +44,7 @@ default void beforeCall(Object obj, Method method, Object[] args) { * @param obj The proxy instance * @param method Method to be replaced * @param args Array of method arguments - * @param original The reference to the original method in case it is necessary to - * instrument its result. + * @param original The reference to the original method in case it is necessary to instrument its result. * @return It is expected that the type of the returned argument could be cast to * the returned type of the original method. */ @@ -75,8 +74,8 @@ default void afterCall(Object obj, Method method, Object[] args, Object result) * @param args Array of method arguments * @param e Exception instance thrown by the original method invocation. * @return You could either (re)throw the exception in this callback or - * overwrite the behavior and return a result from it. It is expected that the - * type of the returned argument could be cast to the returned type of the original method. + * overwrite the behavior and return a result from it. It is expected that the + * type of the returned argument could be cast to the returned type of the original method. */ default Object onError(Object obj, Method method, Object[] args, Throwable e) throws Throwable { throw new NotImplementedException(); diff --git a/src/main/java/io/appium/java_client/proxy/NotImplementedException.java b/src/main/java/io/appium/java_client/proxy/NotImplementedException.java index 9c0425d8e..861c114c8 100644 --- a/src/main/java/io/appium/java_client/proxy/NotImplementedException.java +++ b/src/main/java/io/appium/java_client/proxy/NotImplementedException.java @@ -15,5 +15,6 @@ */ package io.appium.java_client.proxy; + public class NotImplementedException extends RuntimeException { } From a775d5ed8b2c36d330d799544727c27c1995feac Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Wed, 2 Nov 2022 20:16:39 +0100 Subject: [PATCH 5/7] more? --- src/main/java/io/appium/java_client/ios/IOSDriver.java | 3 --- .../java/io/appium/java_client/proxy/MethodCallListener.java | 3 +-- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/main/java/io/appium/java_client/ios/IOSDriver.java b/src/main/java/io/appium/java_client/ios/IOSDriver.java index 89aa7f411..36a0ac2bc 100644 --- a/src/main/java/io/appium/java_client/ios/IOSDriver.java +++ b/src/main/java/io/appium/java_client/ios/IOSDriver.java @@ -23,21 +23,18 @@ import io.appium.java_client.AppiumClientConfig; import io.appium.java_client.AppiumDriver; -import io.appium.java_client.AppiumExecutionMethod; import io.appium.java_client.HasAppStrings; import io.appium.java_client.HasDeviceTime; import io.appium.java_client.HasOnScreenKeyboard; import io.appium.java_client.HidesKeyboard; import io.appium.java_client.HidesKeyboardWithKeyName; import io.appium.java_client.InteractsWithApps; -import io.appium.java_client.MobileCommand; import io.appium.java_client.PullsFiles; import io.appium.java_client.LocksDevice; import io.appium.java_client.PerformsTouchActions; import io.appium.java_client.PushesFiles; import io.appium.java_client.SupportsLegacyAppManagement; import io.appium.java_client.battery.HasBattery; -import io.appium.java_client.remote.AppiumCommandExecutor; import io.appium.java_client.remote.SupportsContextSwitching; import io.appium.java_client.remote.SupportsLocation; import io.appium.java_client.remote.SupportsRotation; diff --git a/src/main/java/io/appium/java_client/proxy/MethodCallListener.java b/src/main/java/io/appium/java_client/proxy/MethodCallListener.java index b35bb3dfd..f0cc1be7a 100644 --- a/src/main/java/io/appium/java_client/proxy/MethodCallListener.java +++ b/src/main/java/io/appium/java_client/proxy/MethodCallListener.java @@ -45,8 +45,7 @@ default void beforeCall(Object obj, Method method, Object[] args) { * @param method Method to be replaced * @param args Array of method arguments * @param original The reference to the original method in case it is necessary to instrument its result. - * @return It is expected that the type of the returned argument could be cast to - * the returned type of the original method. + * @return The type of the returned result should be castable to the returned type of the original method. */ default Object call(Object obj, Method method, Object[] args, Callable original) throws Throwable { throw new NotImplementedException(); From ff9215e6ec4e6d42a92aa594a1c11021b96a1ba1 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Thu, 3 Nov 2022 09:26:37 +0100 Subject: [PATCH 6/7] Use sl4j --- .../io/appium/java_client/proxy/Interceptor.java | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 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 1601f1900..921b86124 100644 --- a/src/main/java/io/appium/java_client/proxy/Interceptor.java +++ b/src/main/java/io/appium/java_client/proxy/Interceptor.java @@ -21,6 +21,8 @@ import net.bytebuddy.implementation.bind.annotation.RuntimeType; import net.bytebuddy.implementation.bind.annotation.SuperCall; import net.bytebuddy.implementation.bind.annotation.This; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.lang.reflect.Method; import java.util.Collection; @@ -29,13 +31,11 @@ import java.util.UUID; import java.util.WeakHashMap; import java.util.concurrent.Callable; -import java.util.logging.Level; -import java.util.logging.Logger; import java.util.stream.Collectors; import java.util.stream.Stream; public class Interceptor { - private static final Logger logger = Logger.getLogger(Interceptor.class.getName()); + 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) @@ -72,7 +72,10 @@ public static Object intercept( } catch (NotImplementedException e) { // ignore } catch (Exception e) { - logger.log(Level.SEVERE, "Got an unexpected error in beforeCall listener", e); + logger.error( + String.format("Got an unexpected error in beforeCall listener of %s.%s method", + self.getClass().getName(), method.getName()), e + ); } }); @@ -115,7 +118,10 @@ public static Object intercept( } catch (NotImplementedException e) { // ignore } catch (Exception e) { - logger.log(Level.SEVERE, "Got an unexpected error in afterCall listener", e); + logger.error( + String.format("Got an unexpected error in afterCall listener of %s.%s method", + self.getClass().getName(), method.getName()), e + ); } }); return endResult; From 1138ca304018565127afb001f0d81a7becb5d371 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Thu, 3 Nov 2022 10:05:12 +0100 Subject: [PATCH 7/7] Tune tests --- docs/The-event_firing.md | 7 +++--- .../io/appium/java_client/AppiumDriver.java | 8 ------- .../java_client/android/AndroidDriver.java | 5 ---- .../io/appium/java_client/ios/IOSDriver.java | 5 ---- .../java_client/proxy/ProxyHelpersTest.java | 23 +++++++++++++++---- 5 files changed, 23 insertions(+), 25 deletions(-) diff --git a/docs/The-event_firing.md b/docs/The-event_firing.md index 9794a11f3..527fddeb8 100644 --- a/docs/The-event_firing.md +++ b/docs/The-event_firing.md @@ -131,11 +131,12 @@ MethodCallListener listener = new MethodCallListener() { acc.append("afterCall ").append(method.getName()).append("\n"); } }; + IOSDriver decoratedDriver = createProxy( IOSDriver.class, - new Objetc[] {new URL("http://localhost:4723/"), new XCUITestOptions()}, - new Class<>[] {URL.class, Capabilities.class}, - Collections.singletonList(listener) + new Object[] {new URL("http://localhost:4723/"), new XCUITestOptions()}, + new Class[] {URL.class, Capabilities.class}, + listener ); decoratedDriver.get("http://example.com/"); diff --git a/src/main/java/io/appium/java_client/AppiumDriver.java b/src/main/java/io/appium/java_client/AppiumDriver.java index b911baff4..450822a69 100644 --- a/src/main/java/io/appium/java_client/AppiumDriver.java +++ b/src/main/java/io/appium/java_client/AppiumDriver.java @@ -68,14 +68,6 @@ public class AppiumDriver extends RemoteWebDriver implements protected final RemoteLocationContext locationContext; private final ExecuteMethod executeMethod; - // Needed for tests - protected AppiumDriver() { - super(); - remoteAddress = null; - executeMethod = new AppiumExecutionMethod(this); - locationContext = new RemoteLocationContext(executeMethod); - } - /** * Creates a new instance based on command {@code executor} and {@code capabilities}. * diff --git a/src/main/java/io/appium/java_client/android/AndroidDriver.java b/src/main/java/io/appium/java_client/android/AndroidDriver.java index 7d169c6c4..92ed19370 100644 --- a/src/main/java/io/appium/java_client/android/AndroidDriver.java +++ b/src/main/java/io/appium/java_client/android/AndroidDriver.java @@ -96,11 +96,6 @@ public class AndroidDriver extends AppiumDriver implements private StringWebSocketClient logcatClient; - // Needed for tests - protected AndroidDriver() { - super(); - } - /** * Creates a new instance based on command {@code executor} and {@code capabilities}. * diff --git a/src/main/java/io/appium/java_client/ios/IOSDriver.java b/src/main/java/io/appium/java_client/ios/IOSDriver.java index 36a0ac2bc..00098b14c 100644 --- a/src/main/java/io/appium/java_client/ios/IOSDriver.java +++ b/src/main/java/io/appium/java_client/ios/IOSDriver.java @@ -85,11 +85,6 @@ public class IOSDriver extends AppiumDriver implements private StringWebSocketClient syslogClient; - // Needed for tests - protected IOSDriver() { - super(); - } - /** * Creates a new instance based on command {@code executor} and {@code capabilities}. * 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 0e79538cf..fc389b6c6 100644 --- a/src/test/java/io/appium/java_client/proxy/ProxyHelpersTest.java +++ b/src/test/java/io/appium/java_client/proxy/ProxyHelpersTest.java @@ -17,11 +17,15 @@ package io.appium.java_client.proxy; import io.appium.java_client.ios.IOSDriver; +import io.appium.java_client.ios.options.XCUITestOptions; import org.junit.jupiter.api.Test; +import org.openqa.selenium.Capabilities; import org.openqa.selenium.remote.RemoteWebDriver; import org.openqa.selenium.remote.UnreachableBrowserException; import java.lang.reflect.Method; +import java.net.MalformedURLException; +import java.net.URL; import java.util.Collections; import java.util.concurrent.Callable; @@ -33,6 +37,15 @@ class ProxyHelpersTest { + public static class FakeIOSDriver extends IOSDriver { + public FakeIOSDriver(URL url, Capabilities caps) { + super(url, caps); + } + + @Override + protected void startSession(Capabilities capabilities) {} + } + @Test void shouldFireBeforeAndAfterEvents() { final StringBuilder acc = new StringBuilder(); @@ -87,7 +100,7 @@ public Object onError(Object obj, Method method, Object[] args, Throwable e) { } @Test - void shouldFireCallEvents() { + void shouldFireCallEvents() throws MalformedURLException { final StringBuilder acc = new StringBuilder(); MethodCallListener listener = new MethodCallListener() { @Override @@ -102,9 +115,11 @@ public Object onError(Object obj, Method method, Object[] args, Throwable e) thr throw e; } }; - IOSDriver driver = createProxy( - IOSDriver.class, - Collections.singletonList(listener) + FakeIOSDriver driver = createProxy( + FakeIOSDriver.class, + new Object[] {new URL("http://localhost:4723/"), new XCUITestOptions()}, + new Class[] {URL.class, Capabilities.class}, + listener ); assertThrows(