From e260b5b4306cc719ed7353963e4d678d9fa3e9e4 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Thu, 7 Nov 2024 17:11:17 -0500 Subject: [PATCH 1/3] [dotnet] Allow `RemoteSessionSettings` to use System.Text.Json values Fixes #14725 --- .../webdriver/Remote/RemoteSessionSettings.cs | 69 ++++++++++--------- .../test/remote/RemoteSessionCreationTests.cs | 64 +++++++++++++++++ 2 files changed, 99 insertions(+), 34 deletions(-) diff --git a/dotnet/src/webdriver/Remote/RemoteSessionSettings.cs b/dotnet/src/webdriver/Remote/RemoteSessionSettings.cs index e3ee3359cca30..2259bbe9b1719 100644 --- a/dotnet/src/webdriver/Remote/RemoteSessionSettings.cs +++ b/dotnet/src/webdriver/Remote/RemoteSessionSettings.cs @@ -22,6 +22,7 @@ using System.Collections.Generic; using System.Globalization; using System.Text.Json; +using System.Text.Json.Nodes; namespace OpenQA.Selenium { @@ -34,7 +35,6 @@ public class RemoteSessionSettings : ICapabilities private const string AlwaysMatchCapabilityName = "alwaysMatch"; private readonly List reservedSettingNames = new List() { FirstMatchCapabilityName, AlwaysMatchCapabilityName }; - private DriverOptions mustMatchDriverOptions; private List firstMatchOptions = new List(); private Dictionary remoteMetadataSettings = new Dictionary(); @@ -60,7 +60,7 @@ public RemoteSessionSettings() /// public RemoteSessionSettings(DriverOptions mustMatchDriverOptions, params DriverOptions[] firstMatchDriverOptions) { - this.mustMatchDriverOptions = mustMatchDriverOptions; + this.MustMatchDriverOptions = mustMatchDriverOptions; foreach (DriverOptions firstMatchOption in firstMatchDriverOptions) { this.AddFirstMatchDriverOption(firstMatchOption); @@ -70,18 +70,12 @@ public RemoteSessionSettings(DriverOptions mustMatchDriverOptions, params Driver /// /// Gets a value indicating the options that must be matched by the remote end to create a session. /// - internal DriverOptions MustMatchDriverOptions - { - get { return this.mustMatchDriverOptions; } - } + internal DriverOptions MustMatchDriverOptions { get; private set; } /// /// Gets a value indicating the number of options that may be matched by the remote end to create a session. /// - internal int FirstMatchOptionsCount - { - get { return this.firstMatchOptions.Count; } - } + internal int FirstMatchOptionsCount => this.firstMatchOptions.Count; /// /// Gets the capability value with the specified name. @@ -91,6 +85,7 @@ internal int FirstMatchOptionsCount /// /// The specified capability name is not in the set of capabilities. /// + /// If is null. public object this[string capabilityName] { get @@ -145,7 +140,7 @@ 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)) + if (!IsJsonSerializable(settingValue)) { throw new ArgumentException("Metadata setting value must be JSON serializable.", nameof(settingValue)); } @@ -160,9 +155,9 @@ public void AddMetadataSetting(string settingName, object settingValue) /// The to add to the list of "first matched" options. public void AddFirstMatchDriverOption(DriverOptions options) { - if (mustMatchDriverOptions != null) + if (MustMatchDriverOptions != null) { - DriverOptionsMergeResult mergeResult = mustMatchDriverOptions.GetMergeResult(options); + DriverOptionsMergeResult mergeResult = 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); @@ -197,7 +192,7 @@ public void SetMustMatchDriverOptions(DriverOptions options) } } - this.mustMatchDriverOptions = options; + this.MustMatchDriverOptions = options; } /// @@ -255,7 +250,7 @@ public Dictionary ToDictionary() capabilitiesDictionary[remoteMetadataSetting.Key] = remoteMetadataSetting.Value; } - if (this.mustMatchDriverOptions != null) + if (this.MustMatchDriverOptions != null) { capabilitiesDictionary["alwaysMatch"] = GetAlwaysMatchOptionsAsSerializableDictionary(); } @@ -291,12 +286,12 @@ internal DriverOptions GetFirstMatchDriverOptions(int firstMatchIndex) private IDictionary GetAlwaysMatchOptionsAsSerializableDictionary() { - return this.mustMatchDriverOptions.ToDictionary(); + return this.MustMatchDriverOptions.ToDictionary(); } private List GetFirstMatchOptionsAsSerializableList() { - List optionsMatches = new List(); + List optionsMatches = new List(this.firstMatchOptions.Count); foreach (DriverOptions options in this.firstMatchOptions) { optionsMatches.Add(options.ToDictionary()); @@ -305,34 +300,42 @@ private List GetFirstMatchOptionsAsSerializableList() return optionsMatches; } - private bool IsJsonSerializable(object arg) + private static bool IsJsonSerializable(object arg) { - IEnumerable argAsEnumerable = arg as IEnumerable; - IDictionary argAsDictionary = arg as IDictionary; + if (arg is null) + { + return true; + } - if (arg is string || arg is float || arg is double || arg is int || arg is long || arg is bool || arg == null) + if (arg is string or float or double or int or long or bool) { return true; } - else if (argAsDictionary != null) + + if (arg is JsonNode or JsonElement) { - foreach (object key in argAsDictionary.Keys) + return true; + } + + if (arg is IDictionary argAsDictionary) + { + foreach (DictionaryEntry item in argAsDictionary) { - if (!(key is string)) + if (item.Key is not string) { return false; } - } - foreach (object value in argAsDictionary.Values) - { - if (!IsJsonSerializable(value)) + if (!IsJsonSerializable(item.Value)) { return false; } } + + return true; } - else if (argAsEnumerable != null) + + if (arg is IEnumerable argAsEnumerable) { foreach (object item in argAsEnumerable) { @@ -341,13 +344,11 @@ private bool IsJsonSerializable(object arg) return false; } } - } - else - { - return false; + + return true; } - return true; + return false; } } } diff --git a/dotnet/test/remote/RemoteSessionCreationTests.cs b/dotnet/test/remote/RemoteSessionCreationTests.cs index 22567b1af90d9..5d02a9700d1e2 100644 --- a/dotnet/test/remote/RemoteSessionCreationTests.cs +++ b/dotnet/test/remote/RemoteSessionCreationTests.cs @@ -1,4 +1,7 @@ using NUnit.Framework; +using System.Collections.Generic; +using System.Text.Json; +using System.Text.Json.Nodes; namespace OpenQA.Selenium.Remote { @@ -49,5 +52,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().And.EqualTo(123)); + + settings.AddMetadataSetting("a", 123f); + Assert.That(settings.HasCapability("a")); + Assert.That(settings.GetCapability("a"), Is.TypeOf().And.EqualTo(123f)); + + settings.AddMetadataSetting("a", 123d); + Assert.That(settings.HasCapability("a")); + Assert.That(settings.GetCapability("a"), Is.TypeOf().And.EqualTo(123d)); + + JsonNode trueName = JsonValue.Create(true); + settings.AddMetadataSetting("a", trueName); + Assert.That(settings.HasCapability("a")); + Assert.That(settings.GetCapability("a"), Is.InstanceOf().And.EqualTo(trueName).Using(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().And.Matches(static left => + { + return left.ValueKind == JsonValueKind.False; + })); + + List intValues = [1, 2, 3]; + settings.AddMetadataSetting("a", intValues); + Assert.That(settings.HasCapability("a")); + Assert.That(settings.GetCapability("a"), Is.TypeOf>().And.EqualTo(intValues)); + + Dictionary dictionaryValues = new Dictionary + { + {"value1", 1 }, + {"value2", 1 }, + }; + settings.AddMetadataSetting("a", dictionaryValues); + Assert.That(settings.HasCapability("a")); + Assert.That(settings.GetCapability("a"), Is.TypeOf>().And.EqualTo(dictionaryValues)); + } } } From 5e06f1eab3e4f3b6d7af1618586c340854603a96 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Sat, 9 Nov 2024 17:08:24 -0500 Subject: [PATCH 2/3] Remove serialization restrictions on `RemoteSessionSettings.AddMetadataSetting` --- .../webdriver/Remote/RemoteSessionSettings.cs | 70 +------------------ 1 file changed, 3 insertions(+), 67 deletions(-) diff --git a/dotnet/src/webdriver/Remote/RemoteSessionSettings.cs b/dotnet/src/webdriver/Remote/RemoteSessionSettings.cs index 165a407e5126a..4ef62e7a4636f 100644 --- a/dotnet/src/webdriver/Remote/RemoteSessionSettings.cs +++ b/dotnet/src/webdriver/Remote/RemoteSessionSettings.cs @@ -116,18 +116,10 @@ public object this[string capabilityName] /// /// The name of the setting to set. /// The value of the setting. - /// - /// 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 that contains JSON-serializable - /// objects, or a where the keys - /// are strings and the values are JSON-serializable. - /// /// - /// 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. + /// If the setting name is null or empty. + /// -or- + /// If one of the reserved names of metadata settings. /// public void AddMetadataSetting(string settingName, object settingValue) { @@ -141,11 +133,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 (!IsJsonSerializable(settingValue)) - { - throw new ArgumentException("Metadata setting value must be JSON serializable.", nameof(settingValue)); - } - this.remoteMetadataSettings[settingName] = settingValue; } @@ -300,56 +287,5 @@ private List GetFirstMatchOptionsAsSerializableList() return optionsMatches; } - - private static bool IsJsonSerializable(object arg) - { - if (arg is null) - { - return true; - } - - if (arg is string or float or double or int or long or bool) - { - return true; - } - - if (arg is JsonNode or JsonElement) - { - return true; - } - - if (arg is IDictionary argAsDictionary) - { - foreach (DictionaryEntry item in argAsDictionary) - { - if (item.Key is not string) - { - return false; - } - - if (!IsJsonSerializable(item.Value)) - { - return false; - } - } - - return true; - } - - if (arg is IEnumerable argAsEnumerable) - { - foreach (object item in argAsEnumerable) - { - if (!IsJsonSerializable(item)) - { - return false; - } - } - - return true; - } - - return false; - } } } From ff86aca40685c5a0d8829018ebd036249ea51a1f Mon Sep 17 00:00:00 2001 From: Michael Render Date: Sat, 9 Nov 2024 18:23:27 -0500 Subject: [PATCH 3/3] Bring back the `RemoteSessionSettings.mustMatchDriverOptions` field --- .../webdriver/Remote/RemoteSessionSettings.cs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/dotnet/src/webdriver/Remote/RemoteSessionSettings.cs b/dotnet/src/webdriver/Remote/RemoteSessionSettings.cs index 4ef62e7a4636f..685d51363e1f9 100644 --- a/dotnet/src/webdriver/Remote/RemoteSessionSettings.cs +++ b/dotnet/src/webdriver/Remote/RemoteSessionSettings.cs @@ -19,11 +19,9 @@ using OpenQA.Selenium.Remote; using System; -using System.Collections; using System.Collections.Generic; using System.Globalization; using System.Text.Json; -using System.Text.Json.Nodes; namespace OpenQA.Selenium { @@ -36,6 +34,7 @@ public class RemoteSessionSettings : ICapabilities private const string AlwaysMatchCapabilityName = "alwaysMatch"; private readonly List reservedSettingNames = new List() { FirstMatchCapabilityName, AlwaysMatchCapabilityName }; + private DriverOptions mustMatchDriverOptions; private List firstMatchOptions = new List(); private Dictionary remoteMetadataSettings = new Dictionary(); @@ -61,7 +60,7 @@ public RemoteSessionSettings() /// public RemoteSessionSettings(DriverOptions mustMatchDriverOptions, params DriverOptions[] firstMatchDriverOptions) { - this.MustMatchDriverOptions = mustMatchDriverOptions; + this.mustMatchDriverOptions = mustMatchDriverOptions; foreach (DriverOptions firstMatchOption in firstMatchDriverOptions) { this.AddFirstMatchDriverOption(firstMatchOption); @@ -71,7 +70,7 @@ public RemoteSessionSettings(DriverOptions mustMatchDriverOptions, params Driver /// /// Gets a value indicating the options that must be matched by the remote end to create a session. /// - internal DriverOptions MustMatchDriverOptions { get; private set; } + internal DriverOptions MustMatchDriverOptions => this.mustMatchDriverOptions; /// /// Gets a value indicating the number of options that may be matched by the remote end to create a session. @@ -143,9 +142,9 @@ public void AddMetadataSetting(string settingName, object settingValue) /// The to add to the list of "first matched" options. 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); @@ -180,7 +179,7 @@ public void SetMustMatchDriverOptions(DriverOptions options) } } - this.MustMatchDriverOptions = options; + this.mustMatchDriverOptions = options; } /// @@ -238,7 +237,7 @@ public Dictionary ToDictionary() capabilitiesDictionary[remoteMetadataSetting.Key] = remoteMetadataSetting.Value; } - if (this.MustMatchDriverOptions != null) + if (this.mustMatchDriverOptions != null) { capabilitiesDictionary["alwaysMatch"] = GetAlwaysMatchOptionsAsSerializableDictionary(); } @@ -274,7 +273,7 @@ internal DriverOptions GetFirstMatchDriverOptions(int firstMatchIndex) private IDictionary GetAlwaysMatchOptionsAsSerializableDictionary() { - return this.MustMatchDriverOptions.ToDictionary(); + return this.mustMatchDriverOptions.ToDictionary(); } private List GetFirstMatchOptionsAsSerializableList()