Only ignore extension caps with object/array values

Author: sbabcoc

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

@@ -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;
 
@@ -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) {
 
@@ -97,18 +88,16 @@ private Boolean initialMatch(Capabilities stereotype, Capabilities capabilities)
         .filter(name -> !name.contains(":"))
         // Platform matching is special, we do it later
         .filter(name -> !"platformName".equalsIgnoreCase(name))
+        .filter(name -> capabilities.getCapability(name) != null)
         .map(
             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));
               }
+              return Objects.equals(
+                  stereotype.getCapability(name), capabilities.getCapability(name));
             })
         .reduce(Boolean::logicalAnd)
         .orElse(true);
@@ -145,27 +134,22 @@ private Boolean platformVersionMatch(Capabilities stereotype, Capabilities capab
   }
 
   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.
-    */
     return stereotype.getCapabilityNames().stream()
         .filter(name -> name.contains(":"))
-        .filter(name -> capabilities.asMap().containsKey(name))
-        .filter(name -> EXTENSION_CAPABILITIES_PREFIXES.stream().noneMatch(name::contains))
+        .filter(name -> capabilities.getCapability(name) != null)
         .map(
             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));
+              }
+              if (capabilities.getCapability(name) instanceof Number
+                  || capabilities.getCapability(name) instanceof Boolean) {
+                return Objects.equals(
+                    stereotype.getCapability(name), capabilities.getCapability(name));
               }
+              return true;
             })
         .reduce(Boolean::logicalAnd)
         .orElse(true);

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<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")) {

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

@@ -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;
@@ -424,7 +426,7 @@ void multipleExtensionPrefixedCapabilitiesDoNotMatchWhenOneIsDifferent() {
   }
 
   @Test
-  void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() {
+  void vendorExtensionPrefixedCapabilitiesWithSimpleValuesAreConsideredForMatching() {
     Capabilities stereotype =
         new ImmutableCapabilities(
             CapabilityType.BROWSER_NAME,
@@ -450,6 +452,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();
   }
 

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()