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 committed Jan 22, 2025
1 parent fd2014d commit 283dae7
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 71 deletions.
85 changes: 35 additions & 50 deletions java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
package org.openqa.selenium.grid.data;

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

Expand All @@ -44,13 +42,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 +67,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 +93,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 +127,37 @@ 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 names do not have the
suffix "options" (case-insensitive). 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 "options" extension capabilities
.filter(name -> !(name.toLowerCase().endsWith("options")))
// ignore capabilities not specified in the request
.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));
// 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));
});
}
}
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
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

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

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 +541,7 @@ void multipleExtensionPrefixedCapabilitiesDoNotMatchWhenOneIsDifferent() {
}

@Test
void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() {
void vendorOptionsCapabilitiesAreIgnoredForMatching() {
Capabilities stereotype =
new ImmutableCapabilities(
CapabilityType.BROWSER_NAME,
Expand All @@ -549,10 +550,10 @@ void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() {
"84",
CapabilityType.PLATFORM_NAME,
Platform.WINDOWS,
"goog:cheese",
"amsterdam",
"ms:fruit",
"mango");
"food:fruitOptions",
"mango",
"food:dairyOptions",
Map.of("cheese", "amsterdam"));

Capabilities capabilities =
new ImmutableCapabilities(
Expand All @@ -562,10 +563,10 @@ void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() {
"84",
CapabilityType.PLATFORM_NAME,
Platform.WINDOWS,
"goog:cheese",
"gouda",
"ms:fruit",
"orange");
"food:fruitOptions",
"orange",
"food:dairyOptions",
Map.of("cheese", "gouda"));
assertThat(slotMatcher.matches(stereotype, capabilities)).isTrue();
}

Expand Down
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 283dae7

Please sign in to comment.