Skip to content

Commit

Permalink
[dotnet] Fully annotate nullability on HttpCommandExecutor (#15110)
Browse files Browse the repository at this point in the history 
* [dotnet] Fully annotate nullability on `HttpCommandExecutor`

* Convert `HttpCommandExecutor` to `Lazy<T>`

* Fix missing `HttpClient.Timeout` set, fix whitespace

* Broaden changes to include HTTP execution in general

* add XML doc about exception in `ICustomDriverCommandExecutor.ExecuteCustomDriverCommand`

* Only log parameter commands at the Trace level

* Do not log command parameters in HttpCommandExecutor at all

* fix push
  • Loading branch information
@RenderMichael
RenderMichael authored Feb 3, 2025
1 parent 00111ed commit c6d887a
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 68 deletions.
12 changes: 7 additions & 5 deletions dotnet/src/webdriver/CommandInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

using System;

#nullable enable

namespace OpenQA.Selenium
{
/// <summary>
Expand All @@ -45,7 +47,7 @@ public override int GetHashCode()
/// </summary>
/// <param name="obj">The <see cref="CommandInfo"/> to compare to this instance.</param>
/// <returns><see langword="true"/> if <paramref name="obj"/> is a <see cref="CommandInfo"/> and its value is the same as this instance; otherwise, <see langword="false"/>. If <paramref name="obj"/> is <see langword="null"/>, the method returns <see langword="false"/>.</returns>
public override bool Equals(object obj)
public override bool Equals(object? obj)
{
return this.Equals(obj as CommandInfo);
}
Expand All @@ -55,15 +57,15 @@ public override bool Equals(object obj)
/// </summary>
/// <param name="other">The <see cref="CommandInfo"/> to compare to this instance.</param>
/// <returns><see langword="true"/> if the value of the <paramref name="other"/> parameter is the same as this instance; otherwise, <see langword="false"/>. If <paramref name="other"/> is <see langword="null"/>, the method returns <see langword="false"/>.</returns>
public bool Equals(CommandInfo other)
public bool Equals(CommandInfo? other)
{
if (other is null)
{
return false;
}

// Optimization for a common success case.
if (Object.ReferenceEquals(this, other))
if (object.ReferenceEquals(this, other))
{
return true;
}
Expand All @@ -86,7 +88,7 @@ public bool Equals(CommandInfo other)
/// <param name="left">The first <see cref="CommandInfo"/> object to compare.</param>
/// <param name="right">The second <see cref="CommandInfo"/> object to compare.</param>
/// <returns><see langword="true"/> if the value of <paramref name="left"/> is the same as the value of <paramref name="right"/>; otherwise, <see langword="false"/>.</returns>
public static bool operator ==(CommandInfo left, CommandInfo right)
public static bool operator ==(CommandInfo? left, CommandInfo? right)
{
if (left is null)
{
Expand All @@ -107,7 +109,7 @@ public bool Equals(CommandInfo other)
/// <param name="left">The first <see cref="CommandInfo"/> object to compare.</param>
/// <param name="right">The second <see cref="CommandInfo"/> object to compare.</param>
/// <returns><see langword="true"/> if the value of <paramref name="left"/> is different from the value of <paramref name="right"/>; otherwise, <see langword="false"/>.</returns>
public static bool operator !=(CommandInfo left, CommandInfo right)
public static bool operator !=(CommandInfo? left, CommandInfo? right)
{
return !(left == right);
}
Expand Down
35 changes: 13 additions & 22 deletions dotnet/src/webdriver/HttpCommandInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
using System;
using System.Globalization;

#nullable enable

namespace OpenQA.Selenium
{
/// <summary>
Expand All @@ -44,42 +46,33 @@ public class HttpCommandInfo : CommandInfo

private const string SessionIdPropertyName = "sessionId";

private string resourcePath;
private string method;

/// <summary>
/// Initializes a new instance of the <see cref="HttpCommandInfo"/> class
/// </summary>
/// <param name="method">Method of the Command</param>
/// <param name="resourcePath">Relative URL path to the resource used to execute the command</param>
public HttpCommandInfo(string method, string resourcePath)
{
this.resourcePath = resourcePath;
this.method = method;
this.ResourcePath = resourcePath;
this.Method = method;
}

/// <summary>
/// Gets the URL representing the path to the resource.
/// </summary>
public string ResourcePath
{
get { return this.resourcePath; }
}
public string ResourcePath { get; }

/// <summary>
/// Gets the HTTP method associated with the command.
/// </summary>
public string Method
{
get { return this.method; }
}
public string Method { get; }

/// <summary>
/// Gets the unique identifier for this command within the scope of its protocol definition
/// </summary>
public override string CommandIdentifier
{
get { return string.Format(CultureInfo.InvariantCulture, "{0} {1}", this.method, this.resourcePath); }
get { return string.Format(CultureInfo.InvariantCulture, "{0} {1}", this.Method, this.ResourcePath); }
}

/// <summary>
Expand All @@ -93,7 +86,7 @@ public override string CommandIdentifier
/// substituted for the tokens in the template.</returns>
public Uri CreateCommandUri(Uri baseUri, Command commandToExecute)
{
string[] urlParts = this.resourcePath.Split(new string[] { "/" }, StringSplitOptions.RemoveEmptyEntries);
string[] urlParts = this.ResourcePath.Split(["/"], StringSplitOptions.RemoveEmptyEntries);
for (int i = 0; i < urlParts.Length; i++)
{
string urlPart = urlParts[i];
Expand All @@ -103,13 +96,11 @@ public Uri CreateCommandUri(Uri baseUri, Command commandToExecute)
}
}

Uri fullUri;
string relativeUrlString = string.Join("/", urlParts);
Uri relativeUri = new Uri(relativeUrlString, UriKind.Relative);
bool uriCreateSucceeded = Uri.TryCreate(baseUri, relativeUri, out fullUri);
if (!uriCreateSucceeded)
if (!Uri.TryCreate(baseUri, relativeUri, out Uri? fullUri))
{
throw new InvalidOperationException(string.Format(CultureInfo.InvariantCulture, "Unable to create URI from base {0} and relative path {1}", baseUri == null ? string.Empty : baseUri.ToString(), relativeUrlString));
throw new InvalidOperationException(string.Format(CultureInfo.InvariantCulture, "Unable to create URI from base {0} and relative path {1}", baseUri?.ToString(), relativeUrlString));
}

return fullUri;
Expand All @@ -133,11 +124,11 @@ private static string GetCommandPropertyValue(string propertyName, Command comma
{
// Extract the URL parameter, and remove it from the parameters dictionary
// so it doesn't get transmitted as a JSON parameter.
if (commandToExecute.Parameters.ContainsKey(propertyName))
if (commandToExecute.Parameters.TryGetValue(propertyName, out var propertyValueObject))
{
if (commandToExecute.Parameters[propertyName] != null)
if (propertyValueObject != null)
{
propertyValue = commandToExecute.Parameters[propertyName].ToString();
propertyValue = propertyValueObject.ToString()!;
commandToExecute.Parameters.Remove(propertyName);
}
}
Expand Down
9 changes: 7 additions & 2 deletions dotnet/src/webdriver/ICommandExecutor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@
// </copyright>

using System;
using System.Diagnostics.CodeAnalysis;
using System.Threading.Tasks;

#nullable enable

namespace OpenQA.Selenium
{
/// <summary>
Expand All @@ -31,15 +34,16 @@ public interface ICommandExecutor : IDisposable
/// Attempts to add a command to the repository of commands known to this executor.
/// </summary>
/// <param name="commandName">The name of the command to attempt to add.</param>
/// <param name="info">The <see cref="CommandInfo"/> describing the commnd to add.</param>
/// <param name="info">The <see cref="CommandInfo"/> describing the command to add.</param>
/// <returns><see langword="true"/> if the new command has been added successfully; otherwise, <see langword="false"/>.</returns>
bool TryAddCommand(string commandName, CommandInfo info);
bool TryAddCommand(string commandName, [NotNullWhen(true)] CommandInfo? info);

/// <summary>
/// Executes a command
/// </summary>
/// <param name="commandToExecute">The command you wish to execute</param>
/// <returns>A response from the browser</returns>
/// <exception cref="ArgumentNullException">If <paramref name="commandToExecute"/> is <see langword="null"/>.</exception>
Response Execute(Command commandToExecute);


Expand All @@ -48,6 +52,7 @@ public interface ICommandExecutor : IDisposable
/// </summary>
/// <param name="commandToExecute">The command you wish to execute</param>
/// <returns>A task object representing the asynchronous operation</returns>
/// <exception cref="ArgumentNullException">If <paramref name="commandToExecute"/> is <see langword="null"/>.</exception>
Task<Response> ExecuteAsync(Command commandToExecute);
}
}
8 changes: 6 additions & 2 deletions dotnet/src/webdriver/ICustomDriverCommandExecutor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
// </copyright>

using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

#nullable enable

namespace OpenQA.Selenium
{
Expand All @@ -32,7 +35,8 @@ public interface ICustomDriverCommandExecutor
/// <param name="driverCommandToExecute">The name of the command to execute. The command name must be registered with the command executor, and must not be a command name known to this driver type.</param>
/// <param name="parameters">A <see cref="Dictionary{K, V}"/> containing the names and values of the parameters of the command.</param>
/// <returns>An object that contains the value returned by the command, if any.</returns>
object ExecuteCustomDriverCommand(string driverCommandToExecute, Dictionary<string, object> parameters);
/// <exception cref="WebDriverException">The command returned an exceptional value.</exception>
object? ExecuteCustomDriverCommand(string driverCommandToExecute, Dictionary<string, object> parameters);

/// <summary>
/// Registers a set of commands to be executed with this driver instance.
Expand All @@ -46,6 +50,6 @@ public interface ICustomDriverCommandExecutor
/// <param name="commandName">The unique name of the command to register.</param>
/// <param name="commandInfo">The <see cref="CommandInfo"/> object describing the command.</param>
/// <returns><see langword="true"/> if the command was registered; otherwise, <see langword="false"/>.</returns>
bool RegisterCustomDriverCommand(string commandName, CommandInfo commandInfo);
bool RegisterCustomDriverCommand(string commandName, [NotNullWhen(true)] CommandInfo? commandInfo);
}
}
5 changes: 4 additions & 1 deletion dotnet/src/webdriver/Remote/DriverServiceCommandExecutor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
// </copyright>

using System;
using System.Diagnostics.CodeAnalysis;
using System.Threading.Tasks;

#nullable enable
Expand Down Expand Up @@ -78,7 +79,7 @@ public DriverServiceCommandExecutor(DriverService service, HttpCommandExecutor c
// get { return this.HttpExecutor.CommandInfoRepository; }
//}

public bool TryAddCommand(string commandName, CommandInfo info)
public bool TryAddCommand(string commandName, [NotNullWhen(true)] CommandInfo? info)
{
return this.HttpExecutor.TryAddCommand(commandName, info);
}
Expand All @@ -94,6 +95,7 @@ public bool TryAddCommand(string commandName, CommandInfo info)
/// </summary>
/// <param name="commandToExecute">The command you wish to execute</param>
/// <returns>A response from the browser</returns>
/// <exception cref="ArgumentNullException">If <paramref name="commandToExecute"/> is <see langword="null"/>.</exception>
public Response Execute(Command commandToExecute)
{
return Task.Run(() => this.ExecuteAsync(commandToExecute)).GetAwaiter().GetResult();
Expand All @@ -104,6 +106,7 @@ public Response Execute(Command commandToExecute)
/// </summary>
/// <param name="commandToExecute">The command you wish to execute</param>
/// <returns>A task object representing the asynchronous operation</returns>
/// <exception cref="ArgumentNullException">If <paramref name="commandToExecute"/> is <see langword="null"/>.</exception>
public async Task<Response> ExecuteAsync(Command commandToExecute)
{
if (commandToExecute == null)
Expand Down
Loading

0 comments on commit c6d887a

Please sign in to comment.