Skip to content

Commit

Permalink
Only ignore extension caps with object/array values
Browse files Browse the repository at this point in the history
  • Loading branch information
@sbabcoc
sbabcoc committed Jan 21, 2025
1 parent 10119a9 commit bd93f46
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 62 deletions.
88 changes: 39 additions & 49 deletions java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
package org.openqa.selenium.grid.data;

import java.io.Serializable;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import org.openqa.selenium.Capabilities;

Expand All @@ -44,13 +44,6 @@
*/
public class DefaultSlotMatcher implements SlotMatcher, Serializable {

/*
List of prefixed extension capabilities we never should try to match, they should be
matched in the Node or in the browser driver.
*/
private static final List<String> EXTENSION_CAPABILITIES_PREFIXES =
Arrays.asList("goog:", "moz:", "ms:", "se:");

@Override
public boolean matches(Capabilities stereotype, Capabilities capabilities) {

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

// At the end, a simple browser, browserVersion and platformName match
boolean browserNameMatch =
(capabilities.getBrowserName() == null || capabilities.getBrowserName().isEmpty())
capabilities.getBrowserName() == null
|| capabilities.getBrowserName().isEmpty()
|| Objects.equals(stereotype.getBrowserName(), capabilities.getBrowserName());
boolean browserVersionMatch =
(capabilities.getBrowserVersion() == null
|| capabilities.getBrowserVersion().isEmpty()
|| Objects.equals(capabilities.getBrowserVersion(), "stable"))
|| browserVersionMatch(
stereotype.getBrowserVersion(), capabilities.getBrowserVersion());
capabilities.getBrowserVersion() == null
|| capabilities.getBrowserVersion().isEmpty()
|| Objects.equals(capabilities.getBrowserVersion(), "stable")
|| browserVersionMatch(stereotype.getBrowserVersion(), capabilities.getBrowserVersion());
boolean platformNameMatch =
capabilities.getPlatformName() == null
|| Objects.equals(stereotype.getPlatformName(), capabilities.getPlatformName())
Expand All @@ -102,21 +95,17 @@ private Boolean initialMatch(Capabilities stereotype, Capabilities capabilities)
.filter(name -> !name.contains(":"))
// Platform matching is special, we do it later
.filter(name -> !"platformName".equalsIgnoreCase(name))
.map(
.filter(name -> capabilities.getCapability(name) != null)
.allMatch(
name -> {
if (capabilities.getCapability(name) instanceof String) {
return stereotype
.getCapability(name)
.toString()
.equalsIgnoreCase(capabilities.getCapability(name).toString());
} else {
return capabilities.getCapability(name) == null
|| Objects.equals(
stereotype.getCapability(name), capabilities.getCapability(name));
if (stereotype.getCapability(name) instanceof String
&& capabilities.getCapability(name) instanceof String) {
return ((String) stereotype.getCapability(name))
.equalsIgnoreCase((String) capabilities.getCapability(name));
}
})
.reduce(Boolean::logicalAnd)
.orElse(true);
return Objects.equals(
stereotype.getCapability(name), capabilities.getCapability(name));
});
}

private Boolean managedDownloadsEnabled(Capabilities stereotype, Capabilities capabilities) {
Expand All @@ -140,39 +129,40 @@ private Boolean platformVersionMatch(Capabilities stereotype, Capabilities capab
*/
return capabilities.getCapabilityNames().stream()
.filter(name -> name.contains("platformVersion"))
.map(
.allMatch(
platformVersionCapName ->
Objects.equals(
stereotype.getCapability(platformVersionCapName),
capabilities.getCapability(platformVersionCapName)))
.reduce(Boolean::logicalAnd)
.orElse(true);
capabilities.getCapability(platformVersionCapName)));
}

private Boolean extensionCapabilitiesMatch(Capabilities stereotype, Capabilities capabilities) {
/*
We match extension capabilities when they are not prefixed with any of the
EXTENSION_CAPABILITIES_PREFIXES items. Also, we match them only when the capabilities
of the new session request contains that specific extension capability.
We match extension capabilities in new session requests whose values are of type
'String', 'Number', or 'Boolean'. We ignore extension capability values of type
'Map' or 'List'. These are forwarded to the matched node for use in configuration,
but are not considered for node matching.
*/
return stereotype.getCapabilityNames().stream()
// examine only extension capabilities
.filter(name -> name.contains(":"))
.filter(name -> capabilities.asMap().containsKey(name))
.filter(name -> EXTENSION_CAPABILITIES_PREFIXES.stream().noneMatch(name::contains))
.map(
// ignore capabilities not specified in the request
.filter(name -> capabilities.getCapability(name) != null)
// ignore capabilities with Map values
.filter(name -> !(capabilities.getCapability(name) instanceof Map))
// ignore capabilities with List values
.filter(name -> !(capabilities.getCapability(name) instanceof List))
.allMatch(
name -> {
if (capabilities.getCapability(name) instanceof String) {
return stereotype
.getCapability(name)
.toString()
.equalsIgnoreCase(capabilities.getCapability(name).toString());
} else {
return capabilities.getCapability(name) == null
|| Objects.equals(
stereotype.getCapability(name), capabilities.getCapability(name));
// evaluate capabilities with String values
if (stereotype.getCapability(name) instanceof String
&& capabilities.getCapability(name) instanceof String) {
return ((String) stereotype.getCapability(name))
.equalsIgnoreCase((String) capabilities.getCapability(name));
}
})
.reduce(Boolean::logicalAnd)
.orElse(true);
// evaluate capabilities with Number or Boolean values
return Objects.equals(
stereotype.getCapability(name), capabilities.getCapability(name));
});
}
}
11 changes: 0 additions & 11 deletions java/src/org/openqa/selenium/grid/node/relay/RelaySessionFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import java.util.logging.Logger;
import org.openqa.selenium.Capabilities;
import org.openqa.selenium.ImmutableCapabilities;
import org.openqa.selenium.MutableCapabilities;
import org.openqa.selenium.SessionNotCreatedException;
import org.openqa.selenium.WebDriverException;
import org.openqa.selenium.grid.data.CreateSessionRequest;
Expand All @@ -50,7 +49,6 @@
import org.openqa.selenium.internal.Debug;
import org.openqa.selenium.internal.Either;
import org.openqa.selenium.internal.Require;
import org.openqa.selenium.remote.CapabilityType;
import org.openqa.selenium.remote.Command;
import org.openqa.selenium.remote.Dialect;
import org.openqa.selenium.remote.DriverCommand;
Expand Down Expand Up @@ -149,15 +147,6 @@ public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sess
"New session request capabilities do not " + "match the stereotype."));
}

// remove browserName capability if 'appium:app' is present as it breaks appium tests when app
// is provided
// they are mutually exclusive
MutableCapabilities filteredStereotype = new MutableCapabilities(stereotype);
if (capabilities.getCapability("appium:app") != null) {
filteredStereotype.setCapability(CapabilityType.BROWSER_NAME, (String) null);
}

capabilities = capabilities.merge(filteredStereotype);
LOG.info("Starting session for " + capabilities);

try (Span span = tracer.getCurrentContext().createSpan("relay_session_factory.apply")) {
Expand Down
34 changes: 33 additions & 1 deletion java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

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

import java.util.List;
import java.util.Map;
import org.junit.jupiter.api.Test;
import org.openqa.selenium.Capabilities;
import org.openqa.selenium.ImmutableCapabilities;
Expand Down Expand Up @@ -540,7 +542,7 @@ void multipleExtensionPrefixedCapabilitiesDoNotMatchWhenOneIsDifferent() {
}

@Test
void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() {
void vendorExtensionPrefixedCapabilitiesWithSimpleValuesAreConsideredForMatching() {
Capabilities stereotype =
new ImmutableCapabilities(
CapabilityType.BROWSER_NAME,
Expand All @@ -566,6 +568,36 @@ void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() {
"gouda",
"ms:fruit",
"orange");
assertThat(slotMatcher.matches(stereotype, capabilities)).isFalse();
}

@Test
void vendorExtensionPrefixedCapabilitiesWithComplexValuesAreIgnoredForMatching() {
Capabilities stereotype =
new ImmutableCapabilities(
CapabilityType.BROWSER_NAME,
"chrome",
CapabilityType.BROWSER_VERSION,
"84",
CapabilityType.PLATFORM_NAME,
Platform.WINDOWS,
"food:dairy",
Map.of("cheese", "amsterdam"),
"food:fruit",
List.of("mango"));

Capabilities capabilities =
new ImmutableCapabilities(
CapabilityType.BROWSER_NAME,
"chrome",
CapabilityType.BROWSER_VERSION,
"84",
CapabilityType.PLATFORM_NAME,
Platform.WINDOWS,
"food:dairy",
Map.of("cheese", "gouda"),
"food:fruit",
List.of("orange"));
assertThat(slotMatcher.matches(stereotype, capabilities)).isTrue();
}

Expand Down
6 changes: 5 additions & 1 deletion java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
import org.openqa.selenium.internal.Either;
import org.openqa.selenium.json.Json;
import org.openqa.selenium.net.NetworkUtils;
import org.openqa.selenium.remote.Browser;
import org.openqa.selenium.safari.SafariDriverInfo;

@SuppressWarnings("DuplicatedCode")
Expand Down Expand Up @@ -148,7 +149,10 @@ boolean isDownloadEnabled(WebDriverInfo driver, String customMsg) {
reported.add(caps);
return Collections.singleton(HelperFactory.create(config, caps));
});
String expected = driver.getDisplayName();
String expected =
"Edge".equals(driver.getDisplayName())
? Browser.EDGE.browserName()
: driver.getDisplayName();

Capabilities found =
reported.stream()
Expand Down

0 comments on commit bd93f46

Please sign in to comment.