From 53304700743fab1e140468ea5903da6a94413ad6 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Tue, 27 Aug 2019 11:56:47 +0200 Subject: [PATCH] fix: Avoid using getSession call for capability values retrieval --- .../io/appium/java_client/AppiumDriver.java | 19 +++++-- .../internal/CapabilityHelpers.java | 52 +++++++++++++++++++ .../java_client/internal/ElementMap.java | 4 +- .../JsonToMobileElementConverter.java | 12 ++--- .../pagefactory/AppiumFieldDecorator.java | 49 ++++++++--------- .../utils/WebDriverUnpackUtility.java | 4 +- .../remote/NewAppiumSessionPayload.java | 2 +- .../ios/IOSNativeWebTapSettingTest.java | 1 + .../widget/tests/AbstractStubWebDriver.java | 16 +++++- .../widget/tests/ExtendedWidgetTest.java | 2 +- .../tests/android/AndroidWidgetTest.java | 2 +- .../widget/tests/ios/XCUITWidgetTest.java | 2 +- .../tests/windows/WindowsWidgetTest.java | 2 +- .../service/local/StartingAppLocallyTest.java | 2 + 14 files changed, 122 insertions(+), 47 deletions(-) create mode 100644 src/main/java/io/appium/java_client/internal/CapabilityHelpers.java diff --git a/src/main/java/io/appium/java_client/AppiumDriver.java b/src/main/java/io/appium/java_client/AppiumDriver.java index c95224da0..c0c1aca0e 100644 --- a/src/main/java/io/appium/java_client/AppiumDriver.java +++ b/src/main/java/io/appium/java_client/AppiumDriver.java @@ -19,9 +19,11 @@ import static com.google.common.base.Preconditions.checkNotNull; import static io.appium.java_client.remote.MobileCapabilityType.PLATFORM_NAME; import static org.apache.commons.lang3.StringUtils.containsIgnoreCase; +import static org.apache.commons.lang3.StringUtils.isBlank; import com.google.common.collect.ImmutableMap; +import io.appium.java_client.internal.CapabilityHelpers; import io.appium.java_client.internal.JsonToMobileElementConverter; import io.appium.java_client.remote.AppiumCommandExecutor; import io.appium.java_client.remote.MobileCapabilityType; @@ -88,7 +90,7 @@ public AppiumDriver(HttpCommandExecutor executor, Capabilities capabilities) { locationContext = new RemoteLocationContext(executeMethod); super.setErrorHandler(errorHandler); this.remoteAddress = executor.getAddressOfRemoteServer(); - this.setElementConverter(new JsonToMobileElementConverter(this, this)); + this.setElementConverter(new JsonToMobileElementConverter(this)); } public AppiumDriver(URL remoteAddress, Capabilities desiredCapabilities) { @@ -314,8 +316,19 @@ public URL getRemoteAddress() { @Override public boolean isBrowser() { - return super.isBrowser() - && !containsIgnoreCase(getContext(), "NATIVE_APP"); + String browserName = CapabilityHelpers.getCapability(getCapabilities(), "browserName", String.class); + if (!isBlank(browserName)) { + try { + return (boolean) executeScript("return !!window.navigator;"); + } catch (WebDriverException ign) { + // ignore + } + } + try { + return !containsIgnoreCase(getContext(), "NATIVE_APP"); + } catch (WebDriverException e) { + return false; + } } @Override diff --git a/src/main/java/io/appium/java_client/internal/CapabilityHelpers.java b/src/main/java/io/appium/java_client/internal/CapabilityHelpers.java new file mode 100644 index 000000000..0cb9ec96c --- /dev/null +++ b/src/main/java/io/appium/java_client/internal/CapabilityHelpers.java @@ -0,0 +1,52 @@ +/* + * 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.internal; + +import org.openqa.selenium.Capabilities; + +import javax.annotation.Nullable; +import java.util.ArrayList; +import java.util.List; + +public class CapabilityHelpers { + public static final String APPIUM_PREFIX = "appium:"; + + /** + * Helper that is used for capability values retrieval. + * Supports both prefixed W3C and "classic" capability names. + * + * @param caps driver caps object + * @param name capability name + * @param expectedType the expected capability type + * @return The retrieved capability value or null if the cap either not present has an unexpected type + */ + @Nullable + public static T getCapability(Capabilities caps, String name, Class expectedType) { + List possibleNames = new ArrayList<>(); + possibleNames.add(name); + if (!name.startsWith(APPIUM_PREFIX)) { + possibleNames.add(APPIUM_PREFIX + name); + } + for (String capName : possibleNames) { + if (caps.getCapability(capName) != null + && expectedType.isAssignableFrom(caps.getCapability(capName).getClass())) { + return expectedType.cast(caps.getCapability(capName)); + } + } + return null; + } +} diff --git a/src/main/java/io/appium/java_client/internal/ElementMap.java b/src/main/java/io/appium/java_client/internal/ElementMap.java index 44c852e77..5522f7cb4 100644 --- a/src/main/java/io/appium/java_client/internal/ElementMap.java +++ b/src/main/java/io/appium/java_client/internal/ElementMap.java @@ -50,12 +50,10 @@ public enum ElementMap { mobileElementMap = builder.build(); } - - private final String platformOrAutomation; private final Class elementClass; - private ElementMap(String platformOrAutomation, Class elementClass) { + ElementMap(String platformOrAutomation, Class elementClass) { this.platformOrAutomation = platformOrAutomation; this.elementClass = elementClass; } diff --git a/src/main/java/io/appium/java_client/internal/JsonToMobileElementConverter.java b/src/main/java/io/appium/java_client/internal/JsonToMobileElementConverter.java index d78b90c7d..22cce7275 100644 --- a/src/main/java/io/appium/java_client/internal/JsonToMobileElementConverter.java +++ b/src/main/java/io/appium/java_client/internal/JsonToMobileElementConverter.java @@ -18,7 +18,7 @@ import static io.appium.java_client.internal.ElementMap.getElementClass; -import io.appium.java_client.HasSessionDetails; +import org.openqa.selenium.Capabilities; import org.openqa.selenium.WebDriverException; import org.openqa.selenium.remote.RemoteWebDriver; import org.openqa.selenium.remote.RemoteWebElement; @@ -41,20 +41,20 @@ public class JsonToMobileElementConverter extends JsonToWebElementConverter { * Creates a new instance based on {@code driver} and object with session details. * * @param driver an instance of {@link RemoteWebDriver} subclass - * @param hasSessionDetails object that has session details */ - public JsonToMobileElementConverter(RemoteWebDriver driver, HasSessionDetails hasSessionDetails) { + public JsonToMobileElementConverter(RemoteWebDriver driver) { super(driver); this.driver = driver; - this.platform = hasSessionDetails.getPlatformName(); - this.automation = hasSessionDetails.getAutomationName(); + Capabilities caps = driver.getCapabilities(); + this.platform = CapabilityHelpers.getCapability(caps, "platformName", String.class); + this.automation = CapabilityHelpers.getCapability(caps, "automationName", String.class); } @Override public Object apply(Object result) { Object toBeReturned = result; if (toBeReturned instanceof RemoteWebElement) { - toBeReturned = newRemoteWebElement(); + toBeReturned = newRemoteWebElement(); ((RemoteWebElement) toBeReturned).setId(((RemoteWebElement) result).getId()); } 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 febee93d6..1e651f00b 100644 --- a/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java +++ b/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java @@ -20,20 +20,22 @@ import static io.appium.java_client.pagefactory.utils.ProxyFactory.getEnhancedProxy; import static io.appium.java_client.pagefactory.utils.WebDriverUnpackUtility.unpackWebDriverFromSearchContext; import static java.time.Duration.ofSeconds; -import static java.util.Optional.ofNullable; import com.google.common.collect.ImmutableList; -import io.appium.java_client.HasSessionDetails; import io.appium.java_client.MobileElement; import io.appium.java_client.android.AndroidElement; +import io.appium.java_client.internal.CapabilityHelpers; import io.appium.java_client.ios.IOSElement; import io.appium.java_client.pagefactory.bys.ContentType; import io.appium.java_client.pagefactory.locator.CacheableLocator; import io.appium.java_client.windows.WindowsElement; +import org.openqa.selenium.Capabilities; +import org.openqa.selenium.HasCapabilities; import org.openqa.selenium.SearchContext; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; +import org.openqa.selenium.remote.RemoteWebDriver; import org.openqa.selenium.remote.RemoteWebElement; import org.openqa.selenium.support.pagefactory.DefaultFieldDecorator; import org.openqa.selenium.support.pagefactory.ElementLocator; @@ -71,30 +73,24 @@ public class AppiumFieldDecorator implements FieldDecorator { private final String automation; private final Duration duration; - /** * Creates field decorator based on {@link SearchContext} and timeout {@code duration}. * - * @param context is an instance of {@link SearchContext} - * It may be the instance of {@link WebDriver} or {@link WebElement} or - * {@link Widget} or some other user's extension/implementation. + * @param context is an instance of {@link SearchContext} + * It may be the instance of {@link WebDriver} or {@link WebElement} or + * {@link Widget} or some other user's extension/implementation. * @param duration is a desired duration of the waiting for an element presence. */ public AppiumFieldDecorator(SearchContext context, Duration duration) { this.webDriver = unpackWebDriverFromSearchContext(context); - HasSessionDetails hasSessionDetails = ofNullable(this.webDriver).map(webDriver -> { - if (!HasSessionDetails.class.isAssignableFrom(webDriver.getClass())) { - return null; - } - return HasSessionDetails.class.cast(webDriver); - }).orElse(null); - if (hasSessionDetails == null) { - platform = null; - automation = null; + if (this.webDriver instanceof HasCapabilities) { + Capabilities caps = ((HasCapabilities) this.webDriver).getCapabilities(); + this.platform = CapabilityHelpers.getCapability(caps, "platformName", String.class); + this.automation = CapabilityHelpers.getCapability(caps, "automationName", String.class); } else { - platform = hasSessionDetails.getPlatformName(); - automation = hasSessionDetails.getAutomationName(); + this.platform = null; + this.automation = null; } this.duration = duration; @@ -115,7 +111,8 @@ protected List proxyForListLocator(ClassLoader ignored, return getEnhancedProxy(ArrayList.class, elementInterceptor); } - @Override protected boolean isDecoratableList(Field field) { + @Override + protected boolean isDecoratableList(Field field) { if (!List.class.isAssignableFrom(field.getType())) { return false; } @@ -148,7 +145,7 @@ public AppiumFieldDecorator(SearchContext context) { * Decorated page object {@code field}. * * @param ignored class loader is ignored by current implementation - * @param field is {@link Field} of page object which is supposed to be decorated. + * @param field is {@link Field} of page object which is supposed to be decorated. * @return a field value or null. */ public Object decorate(ClassLoader ignored, Field field) { @@ -197,19 +194,19 @@ private Object decorateWidget(Field field) { CacheableLocator locator = widgetLocatorFactory.createLocator(field); Map> map = - OverrideWidgetReader.read(widgetType, field, platform); + OverrideWidgetReader.read(widgetType, field, platform); if (isAlist) { return getEnhancedProxy(ArrayList.class, - new WidgetListInterceptor(locator, webDriver, map, widgetType, - duration)); + new WidgetListInterceptor(locator, webDriver, map, widgetType, + duration)); } Constructor constructor = - WidgetConstructorUtil.findConvenientConstructor(widgetType); - return getEnhancedProxy(widgetType, new Class[] {constructor.getParameterTypes()[0]}, - new Object[] {proxyForAnElement(locator)}, - new WidgetInterceptor(locator, webDriver, null, map, duration)); + WidgetConstructorUtil.findConvenientConstructor(widgetType); + return getEnhancedProxy(widgetType, new Class[]{constructor.getParameterTypes()[0]}, + new Object[]{proxyForAnElement(locator)}, + new WidgetInterceptor(locator, webDriver, null, map, duration)); } private WebElement proxyForAnElement(ElementLocator locator) { 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 3fc83d3e4..b15ee6775 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 @@ -82,7 +82,7 @@ public static WebDriver unpackWebDriverFromSearchContext(SearchContext searchCon public static ContentType getCurrentContentType(SearchContext context) { return ofNullable(unpackWebDriverFromSearchContext(context)).map(driver -> { if (HasSessionDetails.class.isAssignableFrom(driver.getClass())) { - HasSessionDetails hasSessionDetails = HasSessionDetails.class.cast(driver); + HasSessionDetails hasSessionDetails = (HasSessionDetails) driver; if (!hasSessionDetails.isBrowser()) { return NATIVE_MOBILE_SPECIFIC; @@ -90,7 +90,7 @@ public static ContentType getCurrentContentType(SearchContext context) { } if (ContextAware.class.isAssignableFrom(driver.getClass())) { //it is desktop browser - ContextAware contextAware = ContextAware.class.cast(driver); + ContextAware contextAware = (ContextAware) driver; String currentContext = contextAware.getContext(); if (containsIgnoreCase(currentContext, NATIVE_APP_PATTERN)) { return NATIVE_MOBILE_SPECIFIC; diff --git a/src/main/java/io/appium/java_client/remote/NewAppiumSessionPayload.java b/src/main/java/io/appium/java_client/remote/NewAppiumSessionPayload.java index 9ec116337..add478a87 100644 --- a/src/main/java/io/appium/java_client/remote/NewAppiumSessionPayload.java +++ b/src/main/java/io/appium/java_client/remote/NewAppiumSessionPayload.java @@ -19,6 +19,7 @@ import static com.google.common.collect.ImmutableMap.of; import static com.google.common.collect.ImmutableMap.toImmutableMap; +import static io.appium.java_client.internal.CapabilityHelpers.APPIUM_PREFIX; import static io.appium.java_client.remote.MobileCapabilityType.FORCE_MJSONWP; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Optional.ofNullable; @@ -88,7 +89,6 @@ public class NewAppiumSessionPayload implements Closeable { .addAll(getAppiumCapabilities(AndroidMobileCapabilityType.class)) .addAll(getAppiumCapabilities(IOSMobileCapabilityType.class)) .addAll(getAppiumCapabilities(YouiEngineCapabilityType.class)).build(); - private static final String APPIUM_PREFIX = "appium:"; private static final String DESIRED_CAPABILITIES = "desiredCapabilities"; private static final String CAPABILITIES = "capabilities"; private static final String REQUIRED_CAPABILITIES = "requiredCapabilities"; diff --git a/src/test/java/io/appium/java_client/ios/IOSNativeWebTapSettingTest.java b/src/test/java/io/appium/java_client/ios/IOSNativeWebTapSettingTest.java index 25f287bc5..25746f91b 100644 --- a/src/test/java/io/appium/java_client/ios/IOSNativeWebTapSettingTest.java +++ b/src/test/java/io/appium/java_client/ios/IOSNativeWebTapSettingTest.java @@ -11,6 +11,7 @@ public class IOSNativeWebTapSettingTest extends BaseSafariTest { @Test public void nativeWebTapSettingTest() { + assertTrue(driver.isBrowser()); driver.get("https://saucelabs.com/test/guinea-pig"); // do a click with nativeWebTap turned on, and assert we get to the right page diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/AbstractStubWebDriver.java b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/AbstractStubWebDriver.java index 218dea51f..7e3d5783a 100644 --- a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/AbstractStubWebDriver.java +++ b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/AbstractStubWebDriver.java @@ -3,7 +3,6 @@ import static com.google.common.collect.ImmutableList.of; import static io.appium.java_client.remote.AutomationName.APPIUM; import static io.appium.java_client.remote.AutomationName.IOS_XCUI_TEST; -import static io.appium.java_client.remote.AutomationName.SELENDROID; import static io.appium.java_client.remote.MobilePlatform.ANDROID; import static io.appium.java_client.remote.MobilePlatform.IOS; import static io.appium.java_client.remote.MobilePlatform.WINDOWS; @@ -11,18 +10,23 @@ import io.appium.java_client.HasSessionDetails; import org.openqa.selenium.By; +import org.openqa.selenium.Capabilities; import org.openqa.selenium.Cookie; +import org.openqa.selenium.HasCapabilities; import org.openqa.selenium.WebDriver; import org.openqa.selenium.logging.Logs; +import org.openqa.selenium.remote.DesiredCapabilities; import org.openqa.selenium.remote.Response; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; -public abstract class AbstractStubWebDriver implements WebDriver, HasSessionDetails { +public abstract class AbstractStubWebDriver implements WebDriver, HasSessionDetails, + HasCapabilities { @Override public Response execute(String driverCommand, Map parameters) { return null; @@ -104,6 +108,14 @@ public Navigation navigate() { return null; } + @Override + public Capabilities getCapabilities() { + Map caps = new HashMap<>(); + caps.put("platformName", getPlatformName()); + caps.put("automationName", getAutomationName()); + return new DesiredCapabilities(caps); + } + @Override public Options manage() { return new Options() { diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/ExtendedWidgetTest.java b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/ExtendedWidgetTest.java index 2523c017b..d8d3a7b60 100644 --- a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/ExtendedWidgetTest.java +++ b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/ExtendedWidgetTest.java @@ -24,7 +24,7 @@ protected ExtendedWidgetTest(ExtendedApp app, WebDriver driver) { public abstract void checkCaseWhenWidgetClassHasNoDeclaredAnnotationButItHasSuperclass(); @Test - public abstract void checkCaseWhenBothWidgetFieldAndClassHaveDelaredAnnotations(); + public abstract void checkCaseWhenBothWidgetFieldAndClassHaveDeclaredAnnotations(); protected static void checkThatLocatorsAreCreatedCorrectly(DefaultStubWidget single, List multiple, By rootLocator, diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/android/AndroidWidgetTest.java b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/android/AndroidWidgetTest.java index 7985e67c4..3b94b21cd 100644 --- a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/android/AndroidWidgetTest.java +++ b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/android/AndroidWidgetTest.java @@ -37,7 +37,7 @@ public void checkCaseWhenWidgetClassHasNoDeclaredAnnotationButItHasSuperclass() } @Override - public void checkCaseWhenBothWidgetFieldAndClassHaveDelaredAnnotations() { + public void checkCaseWhenBothWidgetFieldAndClassHaveDeclaredAnnotations() { checkThatLocatorsAreCreatedCorrectly(((ExtendedApp) app).getExtendedWidgetWithOverriddenLocators(), ((ExtendedApp) app).getExtendedWidgetsWithOverriddenLocators(), AndroidUIAutomator(ANDROID_EXTERNALLY_DEFINED_WIDGET_LOCATOR), diff --git a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/ios/XCUITWidgetTest.java b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/ios/XCUITWidgetTest.java index 6c1e4a1de..56abc937a 100644 --- a/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/ios/XCUITWidgetTest.java +++ b/src/test/java/io/appium/java_client/pagefactory_tests/widget/tests/ios/XCUITWidgetTest.java @@ -37,7 +37,7 @@ public void checkCaseWhenWidgetClassHasNoDeclaredAnnotationButItHasSuperclass() } @Override - public void checkCaseWhenBothWidgetFieldAndClassHaveDelaredAnnotations() { + public void checkCaseWhenBothWidgetFieldAndClassHaveDeclaredAnnotations() { checkThatLocatorsAreCreatedCorrectly(((ExtendedApp) app).getExtendedWidgetWithOverriddenLocators(), ((ExtendedApp) app).getExtendedWidgetsWithOverriddenLocators(), iOSNsPredicateString(XCUIT_EXTERNALLY_DEFINED_WIDGET_LOCATOR), 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 index 5358b29dc..fbd9da9a6 100644 --- 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 @@ -37,7 +37,7 @@ public void checkCaseWhenWidgetClassHasNoDeclaredAnnotationButItHasSuperclass() } @Override - public void checkCaseWhenBothWidgetFieldAndClassHaveDelaredAnnotations() { + public void checkCaseWhenBothWidgetFieldAndClassHaveDeclaredAnnotations() { checkThatLocatorsAreCreatedCorrectly(((ExtendedApp) app).getExtendedWidgetWithOverriddenLocators(), ((ExtendedApp) app).getExtendedWidgetsWithOverriddenLocators(), windowsAutomation(WINDOWS_EXTERNALLY_DEFINED_WIDGET_LOCATOR), diff --git a/src/test/java/io/appium/java_client/service/local/StartingAppLocallyTest.java b/src/test/java/io/appium/java_client/service/local/StartingAppLocallyTest.java index ab7c1d912..1ffecff14 100644 --- a/src/test/java/io/appium/java_client/service/local/StartingAppLocallyTest.java +++ b/src/test/java/io/appium/java_client/service/local/StartingAppLocallyTest.java @@ -18,6 +18,7 @@ import static io.github.bonigarcia.wdm.WebDriverManager.chromedriver; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -197,6 +198,7 @@ public class StartingAppLocallyTest { assertTrue(caps.getCapability(MobileCapabilityType.PLATFORM_NAME) .toString().equalsIgnoreCase(MobilePlatform.IOS)); assertNotEquals(null, caps.getCapability(MobileCapabilityType.DEVICE_NAME)); + assertFalse(driver.isBrowser()); } finally { driver.quit(); }