diff --git a/java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java b/java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java index 3db617bbd5335..c9757fa014397 100644 --- a/java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java +++ b/java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java @@ -39,17 +39,27 @@ * Then the {@code stereotype} must contain the same values. * * - *

One thing to note is that extension capabilities are not considered when matching slots, since - * the matching of these is implementation-specific to each driver. + *

Note that extension capabilities are considered for slot matching, with the following exceptions: + * + *

*/ public class DefaultSlotMatcher implements SlotMatcher, Serializable { /* - List of prefixed extension capabilities we never should try to match, they should be + List of extension capability suffixes we never should try to match, they should be matched in the Node or in the browser driver. */ - private static final List EXTENSION_CAPABILITIES_PREFIXES = - Arrays.asList("goog:", "moz:", "ms:", "se:"); + private static final List EXTENSION_CAPABILITY_SUFFIXES = + Arrays.asList("Options", "options", "loggingPrefs", "debuggerAddress"); @Override public boolean matches(Capabilities stereotype, Capabilities capabilities) { @@ -76,14 +86,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()) @@ -102,21 +112,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) { @@ -140,39 +146,39 @@ 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 prefix "se:" or + one of the reserved suffixes ("options", "Options", "loggingPrefs", or "debuggerAddress"). 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 Selenium extension capabilities + .filter(name -> !name.startsWith("se:")) + // ignore special extension capability suffixes + .filter(name -> EXTENSION_CAPABILITY_SUFFIXES.stream().noneMatch(name::endsWith)) + // 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)); + }); } } diff --git a/java/src/org/openqa/selenium/grid/node/relay/RelaySessionFactory.java b/java/src/org/openqa/selenium/grid/node/relay/RelaySessionFactory.java index fc0c9849ece97..e698904e91bd9 100644 --- a/java/src/org/openqa/selenium/grid/node/relay/RelaySessionFactory.java +++ b/java/src/org/openqa/selenium/grid/node/relay/RelaySessionFactory.java @@ -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; @@ -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; @@ -149,15 +147,6 @@ public Either 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")) { diff --git a/java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java b/java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java index 845a8471d8bde..99e827ad6a56d 100644 --- a/java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java +++ b/java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java @@ -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; @@ -536,7 +537,37 @@ void multipleExtensionPrefixedCapabilitiesDoNotMatchWhenOneIsDifferent() { } @Test - void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() { + void seleniumExtensionCapabilitiesAreIgnoredForMatching() { + Capabilities stereotype = + new ImmutableCapabilities( + CapabilityType.BROWSER_NAME, + "chrome", + CapabilityType.BROWSER_VERSION, + "84", + CapabilityType.PLATFORM_NAME, + Platform.WINDOWS, + "se:cdpVersion", + 1, + "se:downloadsEnabled", + true); + + Capabilities capabilities = + new ImmutableCapabilities( + CapabilityType.BROWSER_NAME, + "chrome", + CapabilityType.BROWSER_VERSION, + "84", + CapabilityType.PLATFORM_NAME, + Platform.WINDOWS, + "se:cdpVersion", + 2, + "se:downloadsEnabled", + false); + assertThat(slotMatcher.matches(stereotype, capabilities)).isTrue(); + } + + @Test + void vendorOptionsCapabilitiesAreIgnoredForMatching() { Capabilities stereotype = new ImmutableCapabilities( CapabilityType.BROWSER_NAME, @@ -545,10 +576,10 @@ void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() { "84", CapabilityType.PLATFORM_NAME, Platform.WINDOWS, - "goog:cheese", - "amsterdam", - "ms:fruit", - "mango"); + "food:fruitOptions", + "mango", + "dairy:options", + Map.of("cheese", "amsterdam")); Capabilities capabilities = new ImmutableCapabilities( @@ -558,10 +589,40 @@ void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() { "84", CapabilityType.PLATFORM_NAME, Platform.WINDOWS, - "goog:cheese", - "gouda", - "ms:fruit", - "orange"); + "food:fruitOptions", + "orange", + "dairy:options", + Map.of("cheese", "gouda")); + assertThat(slotMatcher.matches(stereotype, capabilities)).isTrue(); + } + + @Test + void specialExtensionCapabilitiesAreIgnoredForMatching() { + Capabilities stereotype = + new ImmutableCapabilities( + CapabilityType.BROWSER_NAME, + "chrome", + CapabilityType.BROWSER_VERSION, + "84", + CapabilityType.PLATFORM_NAME, + Platform.WINDOWS, + "food:loggingPrefs", + "mango", + "food:debuggerAddress", + Map.of("cheese", "amsterdam")); + + Capabilities capabilities = + new ImmutableCapabilities( + CapabilityType.BROWSER_NAME, + "chrome", + CapabilityType.BROWSER_VERSION, + "84", + CapabilityType.PLATFORM_NAME, + Platform.WINDOWS, + "food:loggingPrefs", + "orange", + "food:debuggerAddress", + Map.of("cheese", "gouda")); assertThat(slotMatcher.matches(stereotype, capabilities)).isTrue(); } diff --git a/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java b/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java index 69f1ba30a9ee9..3def22ea8dde3 100644 --- a/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java +++ b/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java @@ -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") @@ -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()