Only ignore extension caps with object/array values

SeleniumHQ · Sep 10, 2024 · ac4802b · ac4802b
1 parent 05bce9b
commit ac4802b
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 69 deletions.
diff --git a/java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java b/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) {
 
@@ -93,25 +84,21 @@ public boolean matches(Capabilities stereotype, Capabilities capabilities) {
 
   private Boolean initialMatch(Capabilities stereotype, Capabilities capabilities) {
     return stereotype.getCapabilityNames().stream()
-        // Matching of extension capabilities is implementation independent. Skip them
-        .filter(name -> !name.contains(":"))
-        // Platform matching is special, we do it later
-        .filter(name -> !"platformName".equalsIgnoreCase(name))
-        .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));
-              }
-            })
-        .reduce(Boolean::logicalAnd)
-        .orElse(true);
+      // Matching of extension capabilities is implementation independent. Skip them
+      .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 (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);
   }
 
   private Boolean managedDownloadsEnabled(Capabilities stereotype, Capabilities capabilities) {
@@ -145,29 +132,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))
-        .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));
-              }
-            })
-        .reduce(Boolean::logicalAnd)
-        .orElse(true);
+      .filter(name -> name.contains(":"))
+      .filter(name -> capabilities.getCapability(name) != null)
+      .map(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);
   }
 }
diff --git 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<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")) {

diff --git a/java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java b/java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java
@@ -25,6 +25,9 @@
 import org.openqa.selenium.Platform;
 import org.openqa.selenium.remote.CapabilityType;
 
+import java.util.List;
+import java.util.Map;
+
 class DefaultSlotMatcherTest {
 
   private final DefaultSlotMatcher slotMatcher = new DefaultSlotMatcher();
@@ -433,10 +436,10 @@ void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() {
             "84",
             CapabilityType.PLATFORM_NAME,
             Platform.WINDOWS,
-            "goog:cheese",
-            "amsterdam",
+            "goog:food",
+            Map.of("cheese", "amsterdam"),
             "ms:fruit",
-            "mango");
+            List.of("mango"));
 
     Capabilities capabilities =
         new ImmutableCapabilities(
@@ -446,10 +449,10 @@ void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() {
             "84",
             CapabilityType.PLATFORM_NAME,
             Platform.WINDOWS,
-            "goog:cheese",
-            "gouda",
+            "goog:food",
+            Map.of("cheese", "gouda"),
             "ms:fruit",
-            "orange");
+            List.of("orange"));
     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
@@ -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,7 @@ 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()