Make immutable capabilities be based on an immutable data store

Author: shs96c

java/client/src/org/openqa/selenium/ImmutableCapabilities.java

@@ -19,39 +19,85 @@
 
 import org.openqa.selenium.internal.Require;
 
+import java.util.Collections;
 import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.TreeMap;
+
+import static org.openqa.selenium.SharedCapabilitiesMethods.setCapability;
 
 public class ImmutableCapabilities implements Capabilities {
 
-  private MutableCapabilities delegate = new MutableCapabilities();
+  private final Map<String, Object> delegate;
 
   public ImmutableCapabilities() {
+    this.delegate = Collections.emptyMap();
   }
 
   public ImmutableCapabilities(String k, Object v) {
-    delegate.setCapability(k, v);
+    Require.nonNull("Capability", k);
+    Require.nonNull("Value", v);
+
+    Map<String, Object> delegate = new TreeMap<>();
+    setCapability(delegate, k, v);
+
+    this.delegate = Collections.unmodifiableMap(delegate);
   }
 
   public ImmutableCapabilities(String k1, Object v1, String k2, Object v2) {
-    delegate.setCapability(k1, v1);
-    delegate.setCapability(k2, v2);
+    Require.nonNull("First capability", k1);
+    Require.nonNull("First value", v1);
+    Require.nonNull("Second capability", k2);
+    Require.nonNull("Second value", v2);
+
+    Map<String, Object> delegate = new TreeMap<>();
+
+    setCapability(delegate, k1, v1);
+    setCapability(delegate, k2, v2);
+
+    this.delegate = Collections.unmodifiableMap(delegate);
   }
 
   public ImmutableCapabilities(String k1, Object v1, String k2, Object v2, String k3, Object v3) {
-    delegate.setCapability(k1, v1);
-    delegate.setCapability(k2, v2);
-    delegate.setCapability(k3, v3);
+    Require.nonNull("First capability", k1);
+    Require.nonNull("First value", v1);
+    Require.nonNull("Second capability", k2);
+    Require.nonNull("Second value", v2);
+    Require.nonNull("Third capability", k3);
+    Require.nonNull("Third value", v3);
+
+    Map<String, Object> delegate = new TreeMap<>();
+
+    setCapability(delegate, k1, v1);
+    setCapability(delegate, k2, v2);
+    setCapability(delegate, k3, v3);
+
+    this.delegate = Collections.unmodifiableMap(delegate);
   }
 
   public ImmutableCapabilities(
       String k1, Object v1,
       String k2, Object v2,
       String k3, Object v3,
       String k4, Object v4) {
-    delegate.setCapability(k1, v1);
-    delegate.setCapability(k2, v2);
-    delegate.setCapability(k3, v3);
-    delegate.setCapability(k4, v4);
+    Require.nonNull("First capability", k1);
+    Require.nonNull("First value", v1);
+    Require.nonNull("Second capability", k2);
+    Require.nonNull("Second value", v2);
+    Require.nonNull("Third capability", k3);
+    Require.nonNull("Third value", v3);
+    Require.nonNull("Fourth capability", k4);
+    Require.nonNull("Fourth value", v4);
+
+    Map<String, Object> delegate = new TreeMap<>();
+
+    setCapability(delegate, k1, v1);
+    setCapability(delegate, k2, v2);
+    setCapability(delegate, k3, v3);
+    setCapability(delegate, k4, v4);
+
+    this.delegate = Collections.unmodifiableMap(delegate);
   }
 
   public ImmutableCapabilities(
@@ -60,34 +106,67 @@ public ImmutableCapabilities(
       String k3, Object v3,
       String k4, Object v4,
       String k5, Object v5) {
-    delegate.setCapability(k1, v1);
-    delegate.setCapability(k2, v2);
-    delegate.setCapability(k3, v3);
-    delegate.setCapability(k4, v4);
-    delegate.setCapability(k5, v5);
+    Require.nonNull("First capability", k1);
+    Require.nonNull("First value", v1);
+    Require.nonNull("Second capability", k2);
+    Require.nonNull("Second value", v2);
+    Require.nonNull("Third capability", k3);
+    Require.nonNull("Third value", v3);
+    Require.nonNull("Fourth capability", k4);
+    Require.nonNull("Fourth value", v4);
+    Require.nonNull("Fifth capability", k5);
+    Require.nonNull("Fifth value", v5);
+
+    Map<String, Object> delegate = new TreeMap<>();
+
+    setCapability(delegate, k1, v1);
+    setCapability(delegate, k2, v2);
+    setCapability(delegate, k3, v3);
+    setCapability(delegate, k4, v4);
+    setCapability(delegate, k5, v5);
+
+    this.delegate = Collections.unmodifiableMap(delegate);
   }
 
   public ImmutableCapabilities(Capabilities other) {
     Require.nonNull("Capabilities", other);
-    other.getCapabilityNames().forEach(name -> delegate.setCapability(name, other.getCapability(name)));
+
+    Map<String, Object> delegate = new TreeMap<>();
+    other.getCapabilityNames().forEach(name -> {
+      Require.nonNull("Capability name", name);
+      Object value = other.getCapability(name);
+      Require.nonNull("Capability value", value);
+
+      setCapability(delegate, name, value);
+    });
+
+    this.delegate = Collections.unmodifiableMap(delegate);
   }
 
   public ImmutableCapabilities(Map<?, ?> capabilities) {
     Require.nonNull("Capabilities", capabilities);
+
+    Map<String, Object> delegate = new TreeMap<>();
     capabilities.forEach((key, value) -> {
-      Require.argument("Key", key).instanceOf(String.class);
-      delegate.setCapability(String.valueOf(key), value);
+      Require.argument("Capability key", key).instanceOf(String.class);
+      Object v = capabilities.get(key);
+      Require.nonNull("Capability value", value);
+
+      setCapability(delegate, (String) key, v);
     });
+
+    this.delegate = Collections.unmodifiableMap(delegate);
   }
 
   @Override
   public Object getCapability(String capabilityName) {
-    return delegate.getCapability(capabilityName);
+    Require.nonNull("Capability name", capabilityName);
+    return delegate.get(capabilityName);
   }
 
   @Override
   public Map<String, Object> asMap() {
-    return delegate.asMap();
+    return delegate;
   }
 
   @Override
@@ -100,15 +179,29 @@ public boolean equals(Object o) {
     if (this == o) {
       return true;
     }
+
     if (!(o instanceof Capabilities)) {
       return false;
     }
-    return delegate.equals(o);
+
+    Capabilities other = (Capabilities) o;
+    Set<String> names = other.getCapabilityNames();
+    if (names.size() != delegate.size()) {
+      return false;
+    }
+
+    for (String name : names) {
+      if (!Objects.equals(delegate.get(name), other.getCapability(name))) {
+        return false;
+      }
+    }
+
+    return true;
   }
 
   @Override
   public String toString() {
-    return delegate.toString();
+    return SharedCapabilitiesMethods.toString(delegate);
   }
 
   public static ImmutableCapabilities copyOf(Capabilities capabilities) {
@@ -120,4 +213,5 @@ public static ImmutableCapabilities copyOf(Capabilities capabilities) {
 
     return new ImmutableCapabilities(capabilities);
   }
+
 }

java/client/src/org/openqa/selenium/MutableCapabilities.java

@@ -18,20 +18,13 @@
 package org.openqa.selenium;
 
 import org.openqa.selenium.internal.Require;
-import org.openqa.selenium.logging.LogLevelMapping;
-import org.openqa.selenium.logging.LoggingPreferences;
 
-import java.util.Collection;
 import java.util.Collections;
-import java.util.Comparator;
 import java.util.HashSet;
-import java.util.IdentityHashMap;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.TreeMap;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
 public class MutableCapabilities implements Capabilities {
 
@@ -109,31 +102,7 @@ public void setCapability(String key, Object value) {
       return;
     }
 
-    if ("loggingPrefs".equals(key) && value instanceof Map) {
-      LoggingPreferences prefs = new LoggingPreferences();
-      @SuppressWarnings("unchecked") Map<String, String> prefsMap = (Map<String, String>) value;
-
-      prefsMap.forEach((pKey, pValue) -> prefs.enable(pKey, LogLevelMapping.toLevel(pValue)));
-      caps.put(key, prefs);
-      return;
-    }
-
-    if ("platform".equals(key) && value instanceof String) {
-      try {
-        caps.put(key, Platform.fromString((String) value));
-      } catch (WebDriverException e) {
-        caps.put(key, value);
-      }
-      return;
-    }
-
-    if ("unexpectedAlertBehaviour".equals(key)) {
-      caps.put("unexpectedAlertBehaviour", value);
-      caps.put("unhandledPromptBehavior", value);
-      return;
-    }
-
-    caps.put(key, value);
+    SharedCapabilitiesMethods.setCapability(caps, key, value);
   }
 
   @Override
@@ -183,49 +152,6 @@ public boolean equals(Object o) {
 
   @Override
   public String toString() {
-    Map<Object, String> seen = new IdentityHashMap<>();
-    return "Capabilities " + abbreviate(seen, caps);
-  }
-
-  private String abbreviate(Map<Object, String> seen, Object stringify) {
-    if (stringify == null) {
-      return "null";
-    }
-
-    StringBuilder value = new StringBuilder();
-
-    if (stringify.getClass().isArray()) {
-      value.append("[");
-      value.append(
-        Stream.of((Object[]) stringify)
-          .map(item -> abbreviate(seen, item))
-          .collect(Collectors.joining(", ")));
-      value.append("]");
-    } else if (stringify instanceof Collection) {
-      value.append("[");
-      value.append(
-        ((Collection<?>) stringify).stream()
-          .map(item -> abbreviate(seen, item))
-          .collect(Collectors.joining(", ")));
-      value.append("]");
-    } else if (stringify instanceof Map) {
-      value.append("{");
-      value.append(
-        ((Map<?, ?>) stringify).entrySet().stream()
-          .sorted(Comparator.comparing(entry -> String.valueOf(entry.getKey())))
-          .map(entry -> String.format("%s: %s", entry.getKey(), abbreviate(seen, entry.getValue())))
-          .collect(Collectors.joining(", ")));
-      value.append("}");
-    } else {
-      String s = String.valueOf(stringify);
-      if (s.length() > 30) {
-        value.append(s, 0, 27).append("...");
-      } else {
-        value.append(s);
-      }
-    }
-
-    seen.put(stringify, value.toString());
-    return value.toString();
+    return SharedCapabilitiesMethods.toString(caps);
   }
 }