[dotnet] Allow RemoteSessionSettings to use any value for metadata (SeleniumHQ#14726)

Author: RenderMichael

dotnet/src/webdriver/Remote/RemoteSessionSettings.cs

@@ -19,7 +19,6 @@
 
 using OpenQA.Selenium.Remote;
 using System;
-using System.Collections;
 using System.Collections.Generic;
 using System.Globalization;
 using System.Text.Json;
@@ -71,18 +70,12 @@ public RemoteSessionSettings(DriverOptions mustMatchDriverOptions, params Driver
         /// <summary>
         /// Gets a value indicating the options that must be matched by the remote end to create a session.
         /// </summary>
-        internal DriverOptions MustMatchDriverOptions
-        {
-            get { return this.mustMatchDriverOptions; }
-        }
+        internal DriverOptions MustMatchDriverOptions => this.mustMatchDriverOptions;
 
         /// <summary>
         /// Gets a value indicating the number of options that may be matched by the remote end to create a session.
         /// </summary>
-        internal int FirstMatchOptionsCount
-        {
-            get { return this.firstMatchOptions.Count; }
-        }
+        internal int FirstMatchOptionsCount => this.firstMatchOptions.Count;
 
         /// <summary>
         /// Gets the capability value with the specified name.
@@ -92,6 +85,7 @@ internal int FirstMatchOptionsCount
         /// <exception cref="ArgumentException">
         /// The specified capability name is not in the set of capabilities.
         /// </exception>
+        /// <exception cref="ArgumentNullException">If <paramref name="capabilityName"/> is null.</exception>
         public object this[string capabilityName]
         {
             get
@@ -121,18 +115,10 @@ public object this[string capabilityName]
         /// </summary>
         /// <param name="settingName">The name of the setting to set.</param>
         /// <param name="settingValue">The value of the setting.</param>
-        /// <remarks>
-        /// The value to be set must be serializable to JSON for transmission
-        /// across the wire to the remote end. To be JSON-serializable, the value
-        /// must be a string, a numeric value, a boolean value, an object that
-        /// implmeents <see cref="IEnumerable"/> that contains JSON-serializable
-        /// objects, or a <see cref="Dictionary{TKey, TValue}"/> where the keys
-        /// are strings and the values are JSON-serializable.
-        /// </remarks>
         /// <exception cref="ArgumentException">
-        /// Thrown if the setting name is null, the empty string, or one of the
-        /// reserved names of metadata settings; or if the setting value is not
-        /// JSON serializable.
+        /// <para>If the setting name is null or empty.</para>
+        /// <para>-or-</para>
+        /// <para>If one of the reserved names of metadata settings.</para>
         /// </exception>
         public void AddMetadataSetting(string settingName, object settingValue)
         {
@@ -146,11 +132,6 @@ public void AddMetadataSetting(string settingName, object settingValue)
                 throw new ArgumentException(string.Format("'{0}' is a reserved name for a metadata setting, and cannot be used as a name.", settingName), nameof(settingName));
             }
 
-            if (!this.IsJsonSerializable(settingValue))
-            {
-                throw new ArgumentException("Metadata setting value must be JSON serializable.", nameof(settingValue));
-            }
-
             this.remoteMetadataSettings[settingName] = settingValue;
         }
 
@@ -161,9 +142,9 @@ public void AddMetadataSetting(string settingName, object settingValue)
         /// <param name="options">The <see cref="DriverOptions"/> to add to the list of "first matched" options.</param>
         public void AddFirstMatchDriverOption(DriverOptions options)
         {
-            if (mustMatchDriverOptions != null)
+            if (this.mustMatchDriverOptions != null)
             {
-                DriverOptionsMergeResult mergeResult = mustMatchDriverOptions.GetMergeResult(options);
+                DriverOptionsMergeResult mergeResult = this.mustMatchDriverOptions.GetMergeResult(options);
                 if (mergeResult.IsMergeConflict)
                 {
                     string msg = string.Format(CultureInfo.InvariantCulture, "You cannot request the same capability in both must-match and first-match capabilities. You are attempting to add a first-match driver options object that defines a capability, '{0}', that is already defined in the must-match driver options.", mergeResult.MergeConflictOptionName);
@@ -297,58 +278,13 @@ private IDictionary<string, object> GetAlwaysMatchOptionsAsSerializableDictionar
 
         private List<object> GetFirstMatchOptionsAsSerializableList()
         {
-            List<object> optionsMatches = new List<object>();
+            List<object> optionsMatches = new List<object>(this.firstMatchOptions.Count);
             foreach (DriverOptions options in this.firstMatchOptions)
             {
                 optionsMatches.Add(options.ToDictionary());
             }
 
             return optionsMatches;
         }
-
-        private bool IsJsonSerializable(object arg)
-        {
-            IEnumerable argAsEnumerable = arg as IEnumerable;
-            IDictionary argAsDictionary = arg as IDictionary;
-
-            if (arg is string || arg is float || arg is double || arg is int || arg is long || arg is bool || arg == null)
-            {
-                return true;
-            }
-            else if (argAsDictionary != null)
-            {
-                foreach (object key in argAsDictionary.Keys)
-                {
-                    if (!(key is string))
-                    {
-                        return false;
-                    }
-                }
-
-                foreach (object value in argAsDictionary.Values)
-                {
-                    if (!IsJsonSerializable(value))
-                    {
-                        return false;
-                    }
-                }
-            }
-            else if (argAsEnumerable != null)
-            {
-                foreach (object item in argAsEnumerable)
-                {
-                    if (!IsJsonSerializable(item))
-                    {
-                        return false;
-                    }
-                }
-            }
-            else
-            {
-                return false;
-            }
-
-            return true;
-        }
     }
 }

dotnet/test/remote/RemoteSessionCreationTests.cs

@@ -18,6 +18,9 @@
 // </copyright>
 
 using NUnit.Framework;
+using System.Collections.Generic;
+using System.Text.Json;
+using System.Text.Json.Nodes;
 
 namespace OpenQA.Selenium.Remote
 {
@@ -68,5 +71,66 @@ public void CreateEdgeRemoteSession()
                 edge.Quit();
             }
         }
+
+        [Test]
+        public void ShouldSetRemoteSessionSettingsMetadata()
+        {
+            var settings = new RemoteSessionSettings();
+
+            Assert.That(settings.HasCapability("a"), Is.False);
+
+            settings.AddMetadataSetting("a", null);
+            Assert.That(settings.HasCapability("a"));
+            Assert.That(settings.GetCapability("a"), Is.Null);
+
+            settings.AddMetadataSetting("a", true);
+            Assert.That(settings.HasCapability("a"));
+            Assert.That(settings.GetCapability("a"), Is.True);
+
+            settings.AddMetadataSetting("a", false);
+            Assert.That(settings.HasCapability("a"));
+            Assert.That(settings.GetCapability("a"), Is.False);
+
+            settings.AddMetadataSetting("a", 123);
+            Assert.That(settings.HasCapability("a"));
+            Assert.That(settings.GetCapability("a"), Is.TypeOf<int>().And.EqualTo(123));
+
+            settings.AddMetadataSetting("a", 123f);
+            Assert.That(settings.HasCapability("a"));
+            Assert.That(settings.GetCapability("a"), Is.TypeOf<float>().And.EqualTo(123f));
+
+            settings.AddMetadataSetting("a", 123d);
+            Assert.That(settings.HasCapability("a"));
+            Assert.That(settings.GetCapability("a"), Is.TypeOf<double>().And.EqualTo(123d));
+
+            JsonNode trueName = JsonValue.Create(true);
+            settings.AddMetadataSetting("a", trueName);
+            Assert.That(settings.HasCapability("a"));
+            Assert.That(settings.GetCapability("a"), Is.InstanceOf<JsonNode>().And.EqualTo(trueName).Using<JsonNode>(JsonNode.DeepEquals));
+
+            var reader = new Utf8JsonReader("false"u8);
+            JsonElement trueElement = JsonElement.ParseValue(ref reader);
+
+            settings.AddMetadataSetting("a", trueElement);
+            Assert.That(settings.HasCapability("a"));
+            Assert.That(settings.GetCapability("a"), Is.TypeOf<JsonElement>().And.Matches<JsonElement>(static left =>
+            {
+                return left.ValueKind == JsonValueKind.False;
+            }));
+
+            List<int> intValues = [1, 2, 3];
+            settings.AddMetadataSetting("a", intValues);
+            Assert.That(settings.HasCapability("a"));
+            Assert.That(settings.GetCapability("a"), Is.TypeOf<List<int>>().And.EqualTo(intValues));
+
+            Dictionary<string, int> dictionaryValues = new Dictionary<string, int>
+            {
+                {"value1", 1 },
+                {"value2", 1 },
+            };
+            settings.AddMetadataSetting("a", dictionaryValues);
+            Assert.That(settings.HasCapability("a"));
+            Assert.That(settings.GetCapability("a"), Is.TypeOf<Dictionary<string, int>>().And.EqualTo(dictionaryValues));
+        }
     }
 }