Skip to content

Commit 59cdc70

Browse files
VietND96VietND96
committed
[grid] Improve SlotMatcher for Node relay to Appium server with hybrid browser and native app
Signed-off-by: Viet Nguyen Duc <[email protected]>
1 parent 75615d5 commit 59cdc70

File tree

5 files changed

+269
-34
lines changed

5 files changed

+269
-34
lines changed

‎java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java‎

Lines changed: 57 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ public class DefaultSlotMatcher implements SlotMatcher, Serializable {
5050
*/
5151
private static final List<String> EXTENSION_CAPABILITIES_PREFIXES =
5252
Arrays.asList("goog:", "moz:", "ms:", "se:");
53+
public static final List<String> SPECIFIC_RELAY_CAPABILITIES_APP =
54+
Arrays.asList("appium:app", "appium:appPackage", "appium:bundleId");
55+
public static final List<String> MANDATORY_CAPABILITIES =
56+
Arrays.asList("platformName", "browserName", "browserVersion");
5357

5458
@Override
5559
public boolean matches(Capabilities stereotype, Capabilities capabilities) {
@@ -75,33 +79,22 @@ public boolean matches(Capabilities stereotype, Capabilities capabilities) {
7579
}
7680

7781
// At the end, a simple browser, browserVersion and platformName match
78-
boolean browserNameMatch =
79-
(capabilities.getBrowserName() == null || capabilities.getBrowserName().isEmpty())
80-
|| Objects.equals(stereotype.getBrowserName(), capabilities.getBrowserName());
81-
boolean browserVersionMatch =
82-
(capabilities.getBrowserVersion() == null
83-
|| capabilities.getBrowserVersion().isEmpty()
84-
|| Objects.equals(capabilities.getBrowserVersion(), "stable"))
85-
|| browserVersionMatch(
86-
stereotype.getBrowserVersion(), capabilities.getBrowserVersion());
87-
boolean platformNameMatch =
88-
capabilities.getPlatformName() == null
89-
|| Objects.equals(stereotype.getPlatformName(), capabilities.getPlatformName())
90-
|| (stereotype.getPlatformName() != null
91-
&& stereotype.getPlatformName().is(capabilities.getPlatformName()));
92-
return browserNameMatch && browserVersionMatch && platformNameMatch;
93-
}
82+
boolean browserNameMatch = browserNameMatch(stereotype, capabilities);
83+
boolean browserVersionMatch = browserVersionMatch(stereotype, capabilities);
84+
boolean platformNameMatch = platformNameMatch(stereotype, capabilities);
9485

95-
private boolean browserVersionMatch(String stereotype, String capabilities) {
96-
return new SemanticVersionComparator().compare(stereotype, capabilities) == 0;
86+
return browserNameMatch && browserVersionMatch && platformNameMatch;
9787
}
9888

99-
private Boolean initialMatch(Capabilities stereotype, Capabilities capabilities) {
89+
private boolean initialMatch(Capabilities stereotype, Capabilities capabilities) {
10090
return stereotype.getCapabilityNames().stream()
10191
// Matching of extension capabilities is implementation independent. Skip them
10292
.filter(name -> !name.contains(":"))
103-
// Platform matching is special, we do it later
104-
.filter(name -> !"platformName".equalsIgnoreCase(name))
93+
// Mandatory capabilities match is done at the end
94+
.filter(
95+
name ->
96+
MANDATORY_CAPABILITIES.stream()
97+
.noneMatch(mandatory -> mandatory.equalsIgnoreCase(name)))
10598
.map(
10699
name -> {
107100
if (capabilities.getCapability(name) instanceof String) {
@@ -119,7 +112,7 @@ private Boolean initialMatch(Capabilities stereotype, Capabilities capabilities)
119112
.orElse(true);
120113
}
121114

122-
private Boolean managedDownloadsEnabled(Capabilities stereotype, Capabilities capabilities) {
115+
private boolean managedDownloadsEnabled(Capabilities stereotype, Capabilities capabilities) {
123116
// First lets check if user wanted a Node with managed downloads enabled
124117
Object raw = capabilities.getCapability("se:downloadsEnabled");
125118
if (raw == null || !Boolean.parseBoolean(raw.toString())) {
@@ -132,7 +125,7 @@ private Boolean managedDownloadsEnabled(Capabilities stereotype, Capabilities ca
132125
return raw != null && Boolean.parseBoolean(raw.toString());
133126
}
134127

135-
private Boolean platformVersionMatch(Capabilities stereotype, Capabilities capabilities) {
128+
private boolean platformVersionMatch(Capabilities stereotype, Capabilities capabilities) {
136129
/*
137130
This platform version match is not W3C compliant but users can add Appium servers as
138131
Nodes, so we avoid delaying the match until the Slot, which makes the whole matching
@@ -149,7 +142,7 @@ private Boolean platformVersionMatch(Capabilities stereotype, Capabilities capab
149142
.orElse(true);
150143
}
151144

152-
private Boolean extensionCapabilitiesMatch(Capabilities stereotype, Capabilities capabilities) {
145+
private boolean extensionCapabilitiesMatch(Capabilities stereotype, Capabilities capabilities) {
153146
/*
154147
We match extension capabilities when they are not prefixed with any of the
155148
EXTENSION_CAPABILITIES_PREFIXES items. Also, we match them only when the capabilities
@@ -175,4 +168,44 @@ private Boolean extensionCapabilitiesMatch(Capabilities stereotype, Capabilities
175168
.reduce(Boolean::logicalAnd)
176169
.orElse(true);
177170
}
171+
172+
private boolean browserNameMatch(Capabilities stereotype, Capabilities capabilities) {
173+
return (capabilities.getBrowserName() == null || capabilities.getBrowserName().isEmpty())
174+
|| Objects.equals(stereotype.getBrowserName(), capabilities.getBrowserName())
175+
|| specificRelayCapabilitiesAppMatch(capabilities);
176+
}
177+
178+
private boolean browserVersionMatch(String stereotype, String capabilities) {
179+
return new SemanticVersionComparator().compare(stereotype, capabilities) == 0;
180+
}
181+
182+
private boolean browserVersionMatch(Capabilities stereotype, Capabilities capabilities) {
183+
return (capabilities.getBrowserVersion() == null
184+
|| capabilities.getBrowserVersion().isEmpty()
185+
|| Objects.equals(capabilities.getBrowserVersion(), "stable"))
186+
|| browserVersionMatch(stereotype.getBrowserVersion(), capabilities.getBrowserVersion())
187+
|| specificRelayCapabilitiesAppMatch(capabilities);
188+
}
189+
190+
private boolean platformNameMatch(Capabilities stereotype, Capabilities capabilities) {
191+
return capabilities.getPlatformName() == null
192+
|| Objects.equals(stereotype.getPlatformName(), capabilities.getPlatformName())
193+
|| (stereotype.getPlatformName() != null
194+
&& stereotype.getPlatformName().is(capabilities.getPlatformName()));
195+
}
196+
197+
public static boolean specificRelayCapabilitiesAppMatch(Capabilities capabilities) {
198+
/*
199+
This match is specific for the Relay capabilities that are related to the Appium server for native application.
200+
- If browserName is defined then we always assume it’s a hybrid browser session, so no app-related caps should be provided
201+
- If app is provided then the assumption is that app should be fetched from somewhere first and then installed on the destination device
202+
- If only appPackage is provided for uiautomator2 driver or bundleId for xcuitest then the assumption is we want to automate an app that is already installed on the device under test
203+
- If both (app and appPackage) or (app and bundleId). This will then save some small-time for the driver as by default it tries to autodetect these values anyway by analyzing the fetched package’s manifest
204+
*/
205+
return SPECIFIC_RELAY_CAPABILITIES_APP.stream()
206+
.anyMatch(
207+
name ->
208+
capabilities.getCapability(name) != null
209+
&& !capabilities.getCapability(name).toString().isEmpty());
210+
}
178211
}

‎java/src/org/openqa/selenium/grid/node/relay/RelaySessionFactory.java‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
package org.openqa.selenium.grid.node.relay;
1919

20+
import static org.openqa.selenium.grid.data.DefaultSlotMatcher.specificRelayCapabilitiesAppMatch;
2021
import static org.openqa.selenium.remote.RemoteTags.CAPABILITIES;
2122
import static org.openqa.selenium.remote.RemoteTags.CAPABILITIES_EVENT;
2223
import static org.openqa.selenium.remote.tracing.AttributeKey.DOWNSTREAM_DIALECT;
@@ -72,7 +73,6 @@
7273
public class RelaySessionFactory implements SessionFactory {
7374

7475
private static final Logger LOG = Logger.getLogger(RelaySessionFactory.class.getName());
75-
7676
private final Tracer tracer;
7777
private final HttpClient.Factory clientFactory;
7878
private final Duration sessionTimeout;
@@ -140,24 +140,30 @@ public boolean test(Capabilities capabilities) {
140140
.orElse(false);
141141
}
142142

143+
public Capabilities filterRelayCapabilities(Capabilities capabilities) {
144+
/*
145+
Remove browserName capability if 'appium:app' (or similar based on driver) is present as it breaks appium tests when app is provided
146+
they are mutually exclusive
147+
*/
148+
if (specificRelayCapabilitiesAppMatch(capabilities)) {
149+
MutableCapabilities filteredStereotype = new MutableCapabilities(capabilities);
150+
filteredStereotype.setCapability(CapabilityType.BROWSER_NAME, (String) null);
151+
return filteredStereotype;
152+
}
153+
return capabilities;
154+
}
155+
143156
@Override
144157
public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sessionRequest) {
145158
Capabilities capabilities = sessionRequest.getDesiredCapabilities();
159+
capabilities = filterRelayCapabilities(capabilities);
160+
146161
if (!test(capabilities)) {
147162
return Either.left(
148163
new SessionNotCreatedException(
149164
"New session request capabilities do not " + "match the stereotype."));
150165
}
151166

152-
// remove browserName capability if 'appium:app' is present as it breaks appium tests when app
153-
// is provided
154-
// they are mutually exclusive
155-
MutableCapabilities filteredStereotype = new MutableCapabilities(stereotype);
156-
if (capabilities.getCapability("appium:app") != null) {
157-
filteredStereotype.setCapability(CapabilityType.BROWSER_NAME, (String) null);
158-
}
159-
160-
capabilities = capabilities.merge(filteredStereotype);
161167
LOG.info("Starting session for " + capabilities);
162168

163169
try (Span span = tracer.getCurrentContext().createSpan("relay_session_factory.apply")) {

‎java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,129 @@ void testBrowserVersionMatch() {
5252
assertThat(comparator.compare("130.0", "131")).isEqualTo(-1);
5353
}
5454

55+
@Test
56+
public void testSpecificRelayCapabilitiesAppMatch() {
57+
Capabilities capabilitiesWithApp =
58+
new ImmutableCapabilities(
59+
"appium:app",
60+
"link.to.apk",
61+
"appium:appPackage",
62+
"com.example.app",
63+
"appium:platformVersion",
64+
"15",
65+
"platformName",
66+
"Android",
67+
"appium:automationName",
68+
"uiautomator2");
69+
assertThat(DefaultSlotMatcher.specificRelayCapabilitiesAppMatch(capabilitiesWithApp)).isTrue();
70+
capabilitiesWithApp =
71+
new ImmutableCapabilities(
72+
"browserName",
73+
"chrome",
74+
"appium:platformVersion",
75+
"15",
76+
"platformName",
77+
"Android",
78+
"appium:automationName",
79+
"uiautomator2");
80+
assertThat(DefaultSlotMatcher.specificRelayCapabilitiesAppMatch(capabilitiesWithApp)).isFalse();
81+
}
82+
83+
@Test
84+
public void testRelayNodeMatchByRemovingBrowserNameWhenAppSet() {
85+
/*
86+
Relay node stereotype does not have browserName (where user wants to restrict to run a native app only)
87+
Request capabilities have both browserName (it might initialize by ChromeOptions) and app set
88+
The browserName will be filter out when validating match
89+
*/
90+
Capabilities stereotype =
91+
new ImmutableCapabilities(
92+
CapabilityType.PLATFORM_NAME, Platform.ANDROID, "appium:platformVersion", "14");
93+
Capabilities capabilities =
94+
new ImmutableCapabilities(
95+
CapabilityType.BROWSER_NAME,
96+
"chrome",
97+
CapabilityType.PLATFORM_NAME,
98+
Platform.ANDROID,
99+
"appium:platformVersion",
100+
"14",
101+
"appium:app",
102+
"link.to.apk",
103+
"appium:automationName",
104+
"uiautomator2");
105+
assertThat(slotMatcher.matches(stereotype, capabilities)).isTrue();
106+
}
107+
108+
@Test
109+
public void testRelayNodeNotMatchHybridBrowserVersionWhenStereotypeWithoutBrowserName() {
110+
/*
111+
Relay node 1 has stereotype does not have browserName (where user wants to restrict to run a native app only)
112+
Request capabilities want to run a hybrid app (browserName is set) and app isn't set
113+
Request capabilities should not match the stereotype
114+
Relay node 2 has stereotype with browserName set should match the request capabilities
115+
*/
116+
Capabilities stereotype1 =
117+
new ImmutableCapabilities(
118+
CapabilityType.PLATFORM_NAME, Platform.ANDROID, "appium:platformVersion", "14");
119+
Capabilities capabilities =
120+
new ImmutableCapabilities(
121+
CapabilityType.BROWSER_NAME,
122+
"chrome",
123+
CapabilityType.PLATFORM_NAME,
124+
Platform.ANDROID,
125+
"appium:platformVersion",
126+
"14",
127+
"appium:automationName",
128+
"uiautomator2");
129+
assertThat(slotMatcher.matches(stereotype1, capabilities)).isFalse();
130+
Capabilities stereotype2 =
131+
new ImmutableCapabilities(
132+
CapabilityType.BROWSER_NAME,
133+
"chrome",
134+
CapabilityType.PLATFORM_NAME,
135+
Platform.ANDROID,
136+
"appium:platformVersion",
137+
"14");
138+
assertThat(slotMatcher.matches(stereotype2, capabilities)).isTrue();
139+
}
140+
141+
@Test
142+
public void testRelayNodeNotMatchWhenNonW3CCompliantPlatformVersionSet() {
143+
/*
144+
There are Appium server plugins which allow to “fix” non W3C compliant capabilities by automatically adding `appium:` prefix to them
145+
Relay Node 1: When `platformVersion` is set in both stereotype and capabilities, the non W3C compliant `platformVersion` should be not matched
146+
Relay Node 2: When `platformVersion` is set in stereotype and capabilities, the non W3C compliant `platformVersion` should be matched
147+
*/
148+
Capabilities stereotype1 =
149+
new ImmutableCapabilities(
150+
CapabilityType.BROWSER_NAME,
151+
"chrome",
152+
CapabilityType.PLATFORM_NAME,
153+
Platform.ANDROID,
154+
"platformVersion",
155+
"14");
156+
Capabilities capabilities =
157+
new ImmutableCapabilities(
158+
CapabilityType.BROWSER_NAME,
159+
"chrome",
160+
CapabilityType.PLATFORM_NAME,
161+
Platform.ANDROID,
162+
"platformVersion",
163+
"15",
164+
"appium:automationName",
165+
"uiautomator2");
166+
assertThat(slotMatcher.matches(stereotype1, capabilities)).isFalse();
167+
Capabilities stereotype2 =
168+
new ImmutableCapabilities(
169+
CapabilityType.BROWSER_NAME,
170+
"chrome",
171+
CapabilityType.PLATFORM_NAME,
172+
Platform.ANDROID,
173+
"platformVersion",
174+
"15");
175+
assertThat(slotMatcher.matches(stereotype2, capabilities)).isTrue();
176+
}
177+
55178
@Test
56179
void fullMatch() {
57180
Capabilities stereotype =

‎java/test/org/openqa/selenium/grid/node/relay/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,6 @@ java_test_suite(
1515
"//java/test/org/openqa/selenium/remote/tracing:tracing-support",
1616
artifact("org.junit.jupiter:junit-jupiter-api"),
1717
artifact("org.assertj:assertj-core"),
18+
artifact("org.mockito:mockito-core"),
1819
] + JUNIT5_DEPS,
1920
)

‎java/test/org/openqa/selenium/grid/node/relay/RelaySessionFactoryTest.java‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
// Licensed to the Software Freedom Conservancy (SFC) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The SFC licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.openqa.selenium.grid.node.relay;
19+
20+
import static org.junit.jupiter.api.Assertions.assertEquals;
21+
import static org.mockito.Mockito.when;
22+
23+
import org.junit.jupiter.api.Test;
24+
import org.mockito.Mockito;
25+
import org.openqa.selenium.Capabilities;
26+
import org.openqa.selenium.ImmutableCapabilities;
27+
28+
public class RelaySessionFactoryTest {
29+
30+
// Add the following test method to the `RelaySessionFactoryTest` class
31+
@Test
32+
public void testFilterRelayCapabilities() {
33+
Capabilities capabilitiesWithApp =
34+
new ImmutableCapabilities(
35+
"browserName", "chrome", "platformName", "Android", "appium:app", "/link/to/app.apk");
36+
Capabilities capabilitiesWithAppPackage =
37+
new ImmutableCapabilities(
38+
"browserName",
39+
"chrome",
40+
"platformName",
41+
"Android",
42+
"appium:appPackage",
43+
"com.example.app");
44+
Capabilities capabilitiesWithBundleId =
45+
new ImmutableCapabilities(
46+
"browserName",
47+
"chrome",
48+
"platformName",
49+
"Android",
50+
"appium:bundleId",
51+
"com.example.app");
52+
Capabilities capabilitiesWithoutApp =
53+
new ImmutableCapabilities("browserName", "chrome", "platformName", "Android");
54+
55+
RelaySessionFactory factory = Mockito.mock(RelaySessionFactory.class);
56+
57+
when(factory.filterRelayCapabilities(capabilitiesWithApp)).thenCallRealMethod();
58+
when(factory.filterRelayCapabilities(capabilitiesWithAppPackage)).thenCallRealMethod();
59+
when(factory.filterRelayCapabilities(capabilitiesWithBundleId)).thenCallRealMethod();
60+
when(factory.filterRelayCapabilities(capabilitiesWithoutApp)).thenCallRealMethod();
61+
62+
capabilitiesWithApp = factory.filterRelayCapabilities(capabilitiesWithApp);
63+
capabilitiesWithAppPackage = factory.filterRelayCapabilities(capabilitiesWithAppPackage);
64+
capabilitiesWithBundleId = factory.filterRelayCapabilities(capabilitiesWithBundleId);
65+
capabilitiesWithoutApp = factory.filterRelayCapabilities(capabilitiesWithoutApp);
66+
67+
assertEquals(null, capabilitiesWithApp.getCapability("browserName"));
68+
assertEquals(null, capabilitiesWithAppPackage.getCapability("browserName"));
69+
assertEquals(null, capabilitiesWithBundleId.getCapability("browserName"));
70+
assertEquals("chrome", capabilitiesWithoutApp.getCapability("browserName"));
71+
}
72+
}

0 commit comments

Comments
 (0)