Skip to content

Commit c63ffb7

Browse files
sbabcocsbabcoc
committed
Only ignore extension caps with object/array values
1 parent 4461033 commit c63ffb7

File tree

4 files changed

+77
-62
lines changed

4 files changed

+77
-62
lines changed

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

Lines changed: 39 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@
1818
package org.openqa.selenium.grid.data;
1919

2020
import java.io.Serializable;
21-
import java.util.Arrays;
2221
import java.util.List;
22+
import java.util.Map;
2323
import java.util.Objects;
2424
import org.openqa.selenium.Capabilities;
2525

@@ -44,13 +44,6 @@
4444
*/
4545
public class DefaultSlotMatcher implements SlotMatcher, Serializable {
4646

47-
/*
48-
List of prefixed extension capabilities we never should try to match, they should be
49-
matched in the Node or in the browser driver.
50-
*/
51-
private static final List<String> EXTENSION_CAPABILITIES_PREFIXES =
52-
Arrays.asList("goog:", "moz:", "ms:", "se:");
53-
5447
@Override
5548
public boolean matches(Capabilities stereotype, Capabilities capabilities) {
5649

@@ -76,14 +69,14 @@ public boolean matches(Capabilities stereotype, Capabilities capabilities) {
7669

7770
// At the end, a simple browser, browserVersion and platformName match
7871
boolean browserNameMatch =
79-
(capabilities.getBrowserName() == null || capabilities.getBrowserName().isEmpty())
72+
capabilities.getBrowserName() == null
73+
|| capabilities.getBrowserName().isEmpty()
8074
|| Objects.equals(stereotype.getBrowserName(), capabilities.getBrowserName());
8175
boolean browserVersionMatch =
82-
(capabilities.getBrowserVersion() == null
83-
|| capabilities.getBrowserVersion().isEmpty()
84-
|| Objects.equals(capabilities.getBrowserVersion(), "stable"))
85-
|| browserVersionMatch(
86-
stereotype.getBrowserVersion(), capabilities.getBrowserVersion());
76+
capabilities.getBrowserVersion() == null
77+
|| capabilities.getBrowserVersion().isEmpty()
78+
|| Objects.equals(capabilities.getBrowserVersion(), "stable")
79+
|| browserVersionMatch(stereotype.getBrowserVersion(), capabilities.getBrowserVersion());
8780
boolean platformNameMatch =
8881
capabilities.getPlatformName() == null
8982
|| Objects.equals(stereotype.getPlatformName(), capabilities.getPlatformName())
@@ -102,21 +95,17 @@ private Boolean initialMatch(Capabilities stereotype, Capabilities capabilities)
10295
.filter(name -> !name.contains(":"))
10396
// Platform matching is special, we do it later
10497
.filter(name -> !"platformName".equalsIgnoreCase(name))
105-
.map(
98+
.filter(name -> capabilities.getCapability(name) != null)
99+
.allMatch(
106100
name -> {
107-
if (capabilities.getCapability(name) instanceof String) {
108-
return stereotype
109-
.getCapability(name)
110-
.toString()
111-
.equalsIgnoreCase(capabilities.getCapability(name).toString());
112-
} else {
113-
return capabilities.getCapability(name) == null
114-
|| Objects.equals(
115-
stereotype.getCapability(name), capabilities.getCapability(name));
101+
if (stereotype.getCapability(name) instanceof String
102+
&& capabilities.getCapability(name) instanceof String) {
103+
return ((String) stereotype.getCapability(name))
104+
.equalsIgnoreCase((String) capabilities.getCapability(name));
116105
}
117-
})
118-
.reduce(Boolean::logicalAnd)
119-
.orElse(true);
106+
return Objects.equals(
107+
stereotype.getCapability(name), capabilities.getCapability(name));
108+
});
120109
}
121110

122111
private Boolean managedDownloadsEnabled(Capabilities stereotype, Capabilities capabilities) {
@@ -140,39 +129,40 @@ private Boolean platformVersionMatch(Capabilities stereotype, Capabilities capab
140129
*/
141130
return capabilities.getCapabilityNames().stream()
142131
.filter(name -> name.contains("platformVersion"))
143-
.map(
132+
.allMatch(
144133
platformVersionCapName ->
145134
Objects.equals(
146135
stereotype.getCapability(platformVersionCapName),
147-
capabilities.getCapability(platformVersionCapName)))
148-
.reduce(Boolean::logicalAnd)
149-
.orElse(true);
136+
capabilities.getCapability(platformVersionCapName)));
150137
}
151138

152139
private Boolean extensionCapabilitiesMatch(Capabilities stereotype, Capabilities capabilities) {
153140
/*
154-
We match extension capabilities when they are not prefixed with any of the
155-
EXTENSION_CAPABILITIES_PREFIXES items. Also, we match them only when the capabilities
156-
of the new session request contains that specific extension capability.
141+
We match extension capabilities in new session requests whose values are of type
142+
'String', 'Number', or 'Boolean'. We ignore extension capability values of type
143+
'Map' or 'List'. These are forwarded to the matched node for use in configuration,
144+
but are not considered for node matching.
157145
*/
158146
return stereotype.getCapabilityNames().stream()
147+
// examine only extension capabilities
159148
.filter(name -> name.contains(":"))
160-
.filter(name -> capabilities.asMap().containsKey(name))
161-
.filter(name -> EXTENSION_CAPABILITIES_PREFIXES.stream().noneMatch(name::contains))
162-
.map(
149+
// ignore capabilities not specified in the request
150+
.filter(name -> capabilities.getCapability(name) != null)
151+
// ignore capabilities with Map values
152+
.filter(name -> !(capabilities.getCapability(name) instanceof Map))
153+
// ignore capabilities with List values
154+
.filter(name -> !(capabilities.getCapability(name) instanceof List))
155+
.allMatch(
163156
name -> {
164-
if (capabilities.getCapability(name) instanceof String) {
165-
return stereotype
166-
.getCapability(name)
167-
.toString()
168-
.equalsIgnoreCase(capabilities.getCapability(name).toString());
169-
} else {
170-
return capabilities.getCapability(name) == null
171-
|| Objects.equals(
172-
stereotype.getCapability(name), capabilities.getCapability(name));
157+
// evaluate capabilities with String values
158+
if (stereotype.getCapability(name) instanceof String
159+
&& capabilities.getCapability(name) instanceof String) {
160+
return ((String) stereotype.getCapability(name))
161+
.equalsIgnoreCase((String) capabilities.getCapability(name));
173162
}
174-
})
175-
.reduce(Boolean::logicalAnd)
176-
.orElse(true);
163+
// evaluate capabilities with Number or Boolean values
164+
return Objects.equals(
165+
stereotype.getCapability(name), capabilities.getCapability(name));
166+
});
177167
}
178168
}

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

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
import java.util.logging.Logger;
4141
import org.openqa.selenium.Capabilities;
4242
import org.openqa.selenium.ImmutableCapabilities;
43-
import org.openqa.selenium.MutableCapabilities;
4443
import org.openqa.selenium.SessionNotCreatedException;
4544
import org.openqa.selenium.WebDriverException;
4645
import org.openqa.selenium.grid.data.CreateSessionRequest;
@@ -50,7 +49,6 @@
5049
import org.openqa.selenium.internal.Debug;
5150
import org.openqa.selenium.internal.Either;
5251
import org.openqa.selenium.internal.Require;
53-
import org.openqa.selenium.remote.CapabilityType;
5452
import org.openqa.selenium.remote.Command;
5553
import org.openqa.selenium.remote.Dialect;
5654
import org.openqa.selenium.remote.DriverCommand;
@@ -149,15 +147,6 @@ public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sess
149147
"New session request capabilities do not " + "match the stereotype."));
150148
}
151149

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);
161150
LOG.info("Starting session for " + capabilities);
162151

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

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

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
import static org.assertj.core.api.Assertions.assertThat;
2121

22+
import java.util.List;
23+
import java.util.Map;
2224
import org.junit.jupiter.api.Test;
2325
import org.openqa.selenium.Capabilities;
2426
import org.openqa.selenium.ImmutableCapabilities;
@@ -447,7 +449,7 @@ void multipleExtensionPrefixedCapabilitiesDoNotMatchWhenOneIsDifferent() {
447449
}
448450

449451
@Test
450-
void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() {
452+
void vendorExtensionPrefixedCapabilitiesWithSimpleValuesAreConsideredForMatching() {
451453
Capabilities stereotype =
452454
new ImmutableCapabilities(
453455
CapabilityType.BROWSER_NAME,
@@ -473,6 +475,36 @@ void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() {
473475
"gouda",
474476
"ms:fruit",
475477
"orange");
478+
assertThat(slotMatcher.matches(stereotype, capabilities)).isFalse();
479+
}
480+
481+
@Test
482+
void vendorExtensionPrefixedCapabilitiesWithComplexValuesAreIgnoredForMatching() {
483+
Capabilities stereotype =
484+
new ImmutableCapabilities(
485+
CapabilityType.BROWSER_NAME,
486+
"chrome",
487+
CapabilityType.BROWSER_VERSION,
488+
"84",
489+
CapabilityType.PLATFORM_NAME,
490+
Platform.WINDOWS,
491+
"food:dairy",
492+
Map.of("cheese", "amsterdam"),
493+
"food:fruit",
494+
List.of("mango"));
495+
496+
Capabilities capabilities =
497+
new ImmutableCapabilities(
498+
CapabilityType.BROWSER_NAME,
499+
"chrome",
500+
CapabilityType.BROWSER_VERSION,
501+
"84",
502+
CapabilityType.PLATFORM_NAME,
503+
Platform.WINDOWS,
504+
"food:dairy",
505+
Map.of("cheese", "gouda"),
506+
"food:fruit",
507+
List.of("orange"));
476508
assertThat(slotMatcher.matches(stereotype, capabilities)).isTrue();
477509
}
478510

‎java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
import org.openqa.selenium.internal.Either;
6464
import org.openqa.selenium.json.Json;
6565
import org.openqa.selenium.net.NetworkUtils;
66+
import org.openqa.selenium.remote.Browser;
6667
import org.openqa.selenium.safari.SafariDriverInfo;
6768

6869
@SuppressWarnings("DuplicatedCode")
@@ -148,7 +149,10 @@ boolean isDownloadEnabled(WebDriverInfo driver, String customMsg) {
148149
reported.add(caps);
149150
return Collections.singleton(HelperFactory.create(config, caps));
150151
});
151-
String expected = driver.getDisplayName();
152+
String expected =
153+
"Edge".equals(driver.getDisplayName())
154+
? Browser.EDGE.browserName()
155+
: driver.getDisplayName();
152156

153157
Capabilities found =
154158
reported.stream()

0 commit comments

Comments
 (0)