Simple the changes

Signed-off-by: Viet Nguyen Duc <[email protected]>

Author: VietND96

Diff for: java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java

@@ -70,23 +70,38 @@ public boolean matches(Capabilities stereotype, Capabilities capabilities) {
       return false;
     }
 
-    if (!platformVersionMatch(stereotype, capabilities)) {
+    if (!extensionCapabilitiesMatch(stereotype, capabilities)) {
       return false;
     }
 
-    if (!extensionCapabilitiesMatch(stereotype, capabilities)) {
+    if (!platformVersionMatch(stereotype, capabilities)) {
       return false;
     }
 
     // At the end, a simple browser, browserVersion and platformName match
-    boolean browserNameMatch = browserNameMatch(stereotype, capabilities);
-    boolean browserVersionMatch = browserVersionMatch(stereotype, capabilities);
-    boolean platformNameMatch = platformNameMatch(stereotype, capabilities);
-
+    boolean browserNameMatch =
+        (capabilities.getBrowserName() == null || capabilities.getBrowserName().isEmpty())
+            || Objects.equals(stereotype.getBrowserName(), capabilities.getBrowserName())
+            || matchConditionToRemoveCapability(capabilities);
+    boolean browserVersionMatch =
+        (capabilities.getBrowserVersion() == null
+                || capabilities.getBrowserVersion().isEmpty()
+                || Objects.equals(capabilities.getBrowserVersion(), "stable"))
+            || browserVersionMatch(stereotype.getBrowserVersion(), capabilities.getBrowserVersion())
+            || matchConditionToRemoveCapability(capabilities);
+    boolean platformNameMatch =
+        capabilities.getPlatformName() == null
+            || Objects.equals(stereotype.getPlatformName(), capabilities.getPlatformName())
+            || (stereotype.getPlatformName() != null
+                && stereotype.getPlatformName().is(capabilities.getPlatformName()));
     return browserNameMatch && browserVersionMatch && platformNameMatch;
   }
 
-  private boolean initialMatch(Capabilities stereotype, Capabilities capabilities) {
+  private boolean browserVersionMatch(String stereotype, String capabilities) {
+    return new SemanticVersionComparator().compare(stereotype, capabilities) == 0;
+  }
+
+  private Boolean initialMatch(Capabilities stereotype, Capabilities capabilities) {
     return stereotype.getCapabilityNames().stream()
         // Matching of extension capabilities is implementation independent. Skip them
         .filter(name -> !name.contains(":"))
@@ -112,7 +127,7 @@ private boolean initialMatch(Capabilities stereotype, Capabilities capabilities)
         .orElse(true);
   }
 
-  private boolean managedDownloadsEnabled(Capabilities stereotype, Capabilities capabilities) {
+  private Boolean managedDownloadsEnabled(Capabilities stereotype, Capabilities capabilities) {
     // First lets check if user wanted a Node with managed downloads enabled
     Object raw = capabilities.getCapability("se:downloadsEnabled");
     if (raw == null || !Boolean.parseBoolean(raw.toString())) {
@@ -125,7 +140,7 @@ private boolean managedDownloadsEnabled(Capabilities stereotype, Capabilities ca
     return raw != null && Boolean.parseBoolean(raw.toString());
   }
 
-  private boolean platformVersionMatch(Capabilities stereotype, Capabilities capabilities) {
+  private Boolean platformVersionMatch(Capabilities stereotype, Capabilities capabilities) {
     /*
      This platform version match is not W3C compliant but users can add Appium servers as
      Nodes, so we avoid delaying the match until the Slot, which makes the whole matching
@@ -142,7 +157,7 @@ private boolean platformVersionMatch(Capabilities stereotype, Capabilities capab
         .orElse(true);
   }
 
-  private boolean extensionCapabilitiesMatch(Capabilities stereotype, Capabilities capabilities) {
+  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
@@ -172,32 +187,7 @@ private boolean extensionCapabilitiesMatch(Capabilities stereotype, Capabilities
         .orElse(true);
   }
 
-  private boolean browserNameMatch(Capabilities stereotype, Capabilities capabilities) {
-    return (capabilities.getBrowserName() == null || capabilities.getBrowserName().isEmpty())
-        || Objects.equals(stereotype.getBrowserName(), capabilities.getBrowserName())
-        || specificRelayCapabilitiesAppMatch(capabilities);
-  }
-
-  private boolean browserVersionMatch(String stereotype, String capabilities) {
-    return new SemanticVersionComparator().compare(stereotype, capabilities) == 0;
-  }
-
-  private boolean browserVersionMatch(Capabilities stereotype, Capabilities capabilities) {
-    return (capabilities.getBrowserVersion() == null
-            || capabilities.getBrowserVersion().isEmpty()
-            || Objects.equals(capabilities.getBrowserVersion(), "stable"))
-        || browserVersionMatch(stereotype.getBrowserVersion(), capabilities.getBrowserVersion())
-        || specificRelayCapabilitiesAppMatch(capabilities);
-  }
-
-  private boolean platformNameMatch(Capabilities stereotype, Capabilities capabilities) {
-    return capabilities.getPlatformName() == null
-        || Objects.equals(stereotype.getPlatformName(), capabilities.getPlatformName())
-        || (stereotype.getPlatformName() != null
-            && stereotype.getPlatformName().is(capabilities.getPlatformName()));
-  }
-
-  public static boolean specificRelayCapabilitiesAppMatch(Capabilities capabilities) {
+  public static Boolean matchConditionToRemoveCapability(Capabilities capabilities) {
     /*
     This match is specific for the Relay capabilities that are related to the Appium server for native application.
     - If browserName is defined then we always assume it’s a hybrid browser session, so no app-related caps should be provided

Diff for: java/src/org/openqa/selenium/grid/node/relay/RelaySessionFactory.java

@@ -17,7 +17,7 @@
 
 package org.openqa.selenium.grid.node.relay;
 
-import static org.openqa.selenium.grid.data.DefaultSlotMatcher.specificRelayCapabilitiesAppMatch;
+import static org.openqa.selenium.grid.data.DefaultSlotMatcher.matchConditionToRemoveCapability;
 import static org.openqa.selenium.remote.RemoteTags.CAPABILITIES;
 import static org.openqa.selenium.remote.RemoteTags.CAPABILITIES_EVENT;
 import static org.openqa.selenium.remote.tracing.AttributeKey.DOWNSTREAM_DIALECT;
@@ -145,7 +145,7 @@ public Capabilities filterRelayCapabilities(Capabilities capabilities) {
     Remove browserName capability if 'appium:app' (or similar based on driver) is present as it breaks appium tests when app is provided
     they are mutually exclusive
     */
-    if (specificRelayCapabilitiesAppMatch(capabilities)) {
+    if (matchConditionToRemoveCapability(capabilities)) {
       MutableCapabilities filteredStereotype = new MutableCapabilities(capabilities);
       filteredStereotype.setCapability(CapabilityType.BROWSER_NAME, (String) null);
       return filteredStereotype;

Diff for: java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java

@@ -66,7 +66,7 @@ public void testSpecificRelayCapabilitiesAppMatch() {
             "Android",
             "appium:automationName",
             "uiautomator2");
-    assertThat(DefaultSlotMatcher.specificRelayCapabilitiesAppMatch(capabilitiesWithApp)).isTrue();
+    assertThat(DefaultSlotMatcher.matchConditionToRemoveCapability(capabilitiesWithApp)).isTrue();
     capabilitiesWithApp =
         new ImmutableCapabilities(
             "browserName",
@@ -77,7 +77,7 @@ public void testSpecificRelayCapabilitiesAppMatch() {
             "Android",
             "appium:automationName",
             "uiautomator2");
-    assertThat(DefaultSlotMatcher.specificRelayCapabilitiesAppMatch(capabilitiesWithApp)).isFalse();
+    assertThat(DefaultSlotMatcher.matchConditionToRemoveCapability(capabilitiesWithApp)).isFalse();
   }
 
   @Test