[dotnet] Allow RemoteSessionSettings to use any value for metadata

[RenderMichael]

User description

Fixes #14725

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

The RemoteSessionSettings.AddMetadataSetting method only allows users to pass in "primitive" types.

This is not necessary, as any type that can be serialized to JSON is valid. If a type is somehow not JSON-serializable, then the proper exception will be thrown by the serializer.. It also restricts the use of JSON primitives such as JsonNode and JsonElement from being used.

Motivation and Context

using OpenQA.Selenium;
using System.Text.Json.Nodes;

var remoteSessionSettings = new RemoteSessionSettings();

var trueValue =JsonNode.Parse("true");

remoteSessionSettings.AddMetadataSetting("example", trueValue);

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Enhancement, Tests

---

Description

• Enhanced RemoteSessionSettings to support System.Text.Json.Nodes and System.Text.Json.Elements.
• Replaced the mustMatchDriverOptions field with a property MustMatchDriverOptions for better encapsulation.
• Improved the IsJsonSerializable method to handle JsonNode and JsonElement types, and added null checks.
• Added comprehensive tests to verify the handling of metadata settings in RemoteSessionSettings, ensuring compatibility with various JSON serializable types.

---

Changes walkthrough 📝

	Relevant files
Enhancement	RemoteSessionSettings.cs Enhance RemoteSessionSettings to support System.Text.Json types dotnet/src/webdriver/Remote/RemoteSessionSettings.cs Added support for System.Text.Json.Nodes in RemoteSessionSettings. Replaced mustMatchDriverOptions with a property MustMatchDriverOptions. Enhanced IsJsonSerializable method to handle JsonNode and JsonElement. Improved null handling in IsJsonSerializable. +35/-34
Tests	RemoteSessionCreationTests.cs Add tests for RemoteSessionSettings JSON compatibility      dotnet/test/remote/RemoteSessionCreationTests.cs Added tests for RemoteSessionSettings metadata settings. Verified JSON serializability of various data types. Ensured compatibility with JsonNode and JsonElement. +64/-0

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Performance Concern The IsJsonSerializable method performs recursive checks on collections, which could be inefficient for large or deeply nested structures. Consider implementing a depth limit or using a more efficient serialization check. Potential Bug The AddMetadataSetting method checks if the value is JSON serializable before adding it to the dictionary. However, it's not clear if this check is comprehensive enough for all possible JSON-serializable types, especially custom objects. Test Coverage While the new tests cover various data types, they don't include edge cases or error scenarios. Consider adding tests for invalid inputs or boundary conditions.

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Enhancement	Refactor multiple if statements into a switch expression for improved code clarity and maintainability Consider using a switch expression instead of multiple if statements for better readability and maintainability in the IsJsonSerializable method. dotnet/src/webdriver/Remote/RemoteSessionSettings.cs [303-352] -if (arg is string or float or double or int or long or bool) +return arg switch { - return true; -} + null => true, + string or float or double or int or long or bool => true, + JsonNode or JsonElement => true, + IDictionary argAsDictionary => IsJsonSerializableDictionary(argAsDictionary), + IEnumerable argAsEnumerable => IsJsonSerializableEnumerable(argAsEnumerable), + _ => false +}; -if (arg is JsonNode or JsonElement) -{ - return true; -} +// Implement IsJsonSerializableDictionary and IsJsonSerializableEnumerable methods -if (arg is IDictionary argAsDictionary) -{ - // ... dictionary handling ... -} - -if (arg is IEnumerable argAsEnumerable) -{ - // ... enumerable handling ... -} - -return false; - Suggestion importance[1-10]: 7 Why: The suggestion offers a more concise and readable approach to handling different types in the IsJsonSerializable method. It improves maintainability and aligns with modern C# practices.	7
	Use pattern matching to simplify null checks and improve code readability Consider using pattern matching in the AddFirstMatchDriverOption method to simplify the null check and improve readability. dotnet/src/webdriver/Remote/RemoteSessionSettings.cs [158-164] -if (MustMatchDriverOptions != null) +if (MustMatchDriverOptions is { } mustMatchOptions) { - DriverOptionsMergeResult mergeResult = MustMatchDriverOptions.GetMergeResult(options); + var mergeResult = mustMatchOptions.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); - throw new ArgumentException(msg, nameof(options)); + throw new ArgumentException( + $"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, '{mergeResult.MergeConflictOptionName}', that is already defined in the must-match driver options.", + nameof(options)); } } Apply this suggestion Suggestion importance[1-10]: 6 Why: The suggestion enhances code readability by using pattern matching and string interpolation. It also slightly improves performance by avoiding repeated null checks.	6
	Simplify null check using the null-coalescing operator for more concise code Consider using the null-coalescing operator (??) instead of an if statement for the MustMatchDriverOptions check in the ToDictionary method. dotnet/src/webdriver/Remote/RemoteSessionSettings.cs [253-256] -if (this.MustMatchDriverOptions != null) -{ - capabilitiesDictionary["alwaysMatch"] = GetAlwaysMatchOptionsAsSerializableDictionary(); -} +capabilitiesDictionary["alwaysMatch"] = this.MustMatchDriverOptions != null ? GetAlwaysMatchOptionsAsSerializableDictionary() : null; Apply this suggestion Suggestion importance[1-10]: 4 Why: While the suggestion does make the code more concise, it doesn't significantly improve readability or functionality. The existing code is already clear and straightforward.	4

💡 Need additional feedback ? start a PR chat

[nvborisenko]

Can you please adjust PR description according to the latest changes? Thx.

[RenderMichael]

PR description updated,

[nvborisenko]

Thank you!