[dotnet] [bidi] Extensible additional command data via options#17258
[dotnet] [bidi] Extensible additional command data via options#17258nvborisenko wants to merge 1 commit intoSeleniumHQ:trunkfrom
Conversation
Review Summary by Qodo
WalkthroughsDescription• Add optional extensionData parameter to all BiDi command classes • Enable passing extension data through command options to commands • Update base Command class to support JSON extension data serialization • Add ExtensionData property to CommandOptions base class • Update all module methods to pass extension data from options to commands File Changes1. dotnet/src/webdriver/BiDi/Command.cs
|
Code Review by Qodo
1. Command ctor signature changed
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| public abstract class Command(string method, IDictionary<string, JsonElement>? extensionData) | ||
| { | ||
| protected Command(string method) | ||
| { | ||
| Method = method; | ||
| } | ||
| [JsonPropertyOrder(0)] | ||
| public long Id { get; internal set; } | ||
|
|
||
| [JsonPropertyOrder(1)] | ||
| public string Method { get; } | ||
| public string Method { get; } = method; | ||
|
|
||
| [JsonPropertyOrder(0)] | ||
| public long Id { get; internal set; } | ||
| [JsonExtensionData] | ||
| // IMPORTANT: The name is different from ctor parameter to avoid collision with the JsonExtensionData attribute. | ||
| public IDictionary<string, JsonElement>? JsonExtensionData { get; } = extensionData; |
There was a problem hiding this comment.
1. command ctor signature changed 📘 Rule violation ⚙ Maintainability
The public abstract Command base type removed the prior protected Command(string method) constructor and now requires an additional extensionData parameter, which is a source-breaking change for any consumer-defined derived commands. This violates the requirement to maintain backward-compatible public API/ABI or deprecate before removal.
Agent Prompt
## Issue description
The public `OpenQA.Selenium.BiDi.Command` base type changed constructor shape (removed `protected Command(string method)`), which is a breaking change for consumers who derive their own commands.
## Issue Context
This PR adds extension data support. Keep that support while preserving backward-compatible construction for existing derived types.
## Fix Focus Areas
- dotnet/src/webdriver/BiDi/Command.cs[26-36]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| public abstract record CommandOptions | ||
| { | ||
| public TimeSpan? Timeout { get; init; } | ||
|
|
||
| public JsonObject? ExtensionData { get; init; } = []; |
There was a problem hiding this comment.
2. extensiondata exposes mutable state 📎 Requirement gap ⛯ Reliability
CommandOptions.ExtensionData is JsonObject, which remains mutable after construction, and tests demonstrate mutating it post-instantiation. This violates the requirement that public options types be immutable from the consumer’s perspective.
Agent Prompt
## Issue description
`CommandOptions.ExtensionData` is a mutable `JsonObject`, allowing mutation after options construction (even with an `init` property). This violates the immutability requirement for command options.
## Issue Context
The PR adds extensible command data, but the options surface should not expose mutable state. Consider switching to an immutable/read-only representation (e.g., `IReadOnlyDictionary<string, JsonElement>` backed by `ImmutableDictionary`) and/or copying/freezing data on construction.
## Fix Focus Areas
- dotnet/src/webdriver/BiDi/CommandOptions.cs[24-28]
- dotnet/test/common/BiDi/Session/SessionTests.cs[63-71]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
2 participants
Spec:
At the same time:
🔗 Related Issues
Contributes to #16095
💥 What does this PR do?
This pull request updates the WebDriver BiDi (Bidirectional) command infrastructure to allow passing extension data with command objects. This change adds support for optional extension data (via
JsonObject? extensionData) to various command classes and ensures that the extension data from options is consistently passed through when commands are executed. This makes the command system more flexible and extensible for future enhancements or custom data.These changes collectively improve the extensibility of the BiDi command infrastructure, allowing for richer command customization and future protocol evolution.
🔄 Types of changes