Skip to content

Commit 5d8b940

Browse files
RenderMichaelsandeepsuryaprasad
authored andcommitted
[dotnet] Fully annotate nullability on HttpCommandExecutor (SeleniumHQ#15110)
* [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
1 parent 209beef commit 5d8b940

7 files changed

+87
-68
lines changed

dotnet/src/webdriver/CommandInfo.cs

+7-5
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
using System;
2121

22+
#nullable enable
23+
2224
namespace OpenQA.Selenium
2325
{
2426
/// <summary>
@@ -45,7 +47,7 @@ public override int GetHashCode()
4547
/// </summary>
4648
/// <param name="obj">The <see cref="CommandInfo"/> to compare to this instance.</param>
4749
/// <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>
48-
public override bool Equals(object obj)
50+
public override bool Equals(object? obj)
4951
{
5052
return this.Equals(obj as CommandInfo);
5153
}
@@ -55,15 +57,15 @@ public override bool Equals(object obj)
5557
/// </summary>
5658
/// <param name="other">The <see cref="CommandInfo"/> to compare to this instance.</param>
5759
/// <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>
58-
public bool Equals(CommandInfo other)
60+
public bool Equals(CommandInfo? other)
5961
{
6062
if (other is null)
6163
{
6264
return false;
6365
}
6466

6567
// Optimization for a common success case.
66-
if (Object.ReferenceEquals(this, other))
68+
if (object.ReferenceEquals(this, other))
6769
{
6870
return true;
6971
}
@@ -86,7 +88,7 @@ public bool Equals(CommandInfo other)
8688
/// <param name="left">The first <see cref="CommandInfo"/> object to compare.</param>
8789
/// <param name="right">The second <see cref="CommandInfo"/> object to compare.</param>
8890
/// <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>
89-
public static bool operator ==(CommandInfo left, CommandInfo right)
91+
public static bool operator ==(CommandInfo? left, CommandInfo? right)
9092
{
9193
if (left is null)
9294
{
@@ -107,7 +109,7 @@ public bool Equals(CommandInfo other)
107109
/// <param name="left">The first <see cref="CommandInfo"/> object to compare.</param>
108110
/// <param name="right">The second <see cref="CommandInfo"/> object to compare.</param>
109111
/// <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>
110-
public static bool operator !=(CommandInfo left, CommandInfo right)
112+
public static bool operator !=(CommandInfo? left, CommandInfo? right)
111113
{
112114
return !(left == right);
113115
}

dotnet/src/webdriver/HttpCommandInfo.cs

+13-22
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
using System;
2121
using System.Globalization;
2222

23+
#nullable enable
24+
2325
namespace OpenQA.Selenium
2426
{
2527
/// <summary>
@@ -44,42 +46,33 @@ public class HttpCommandInfo : CommandInfo
4446

4547
private const string SessionIdPropertyName = "sessionId";
4648

47-
private string resourcePath;
48-
private string method;
49-
5049
/// <summary>
5150
/// Initializes a new instance of the <see cref="HttpCommandInfo"/> class
5251
/// </summary>
5352
/// <param name="method">Method of the Command</param>
5453
/// <param name="resourcePath">Relative URL path to the resource used to execute the command</param>
5554
public HttpCommandInfo(string method, string resourcePath)
5655
{
57-
this.resourcePath = resourcePath;
58-
this.method = method;
56+
this.ResourcePath = resourcePath;
57+
this.Method = method;
5958
}
6059

6160
/// <summary>
6261
/// Gets the URL representing the path to the resource.
6362
/// </summary>
64-
public string ResourcePath
65-
{
66-
get { return this.resourcePath; }
67-
}
63+
public string ResourcePath { get; }
6864

6965
/// <summary>
7066
/// Gets the HTTP method associated with the command.
7167
/// </summary>
72-
public string Method
73-
{
74-
get { return this.method; }
75-
}
68+
public string Method { get; }
7669

7770
/// <summary>
7871
/// Gets the unique identifier for this command within the scope of its protocol definition
7972
/// </summary>
8073
public override string CommandIdentifier
8174
{
82-
get { return string.Format(CultureInfo.InvariantCulture, "{0} {1}", this.method, this.resourcePath); }
75+
get { return string.Format(CultureInfo.InvariantCulture, "{0} {1}", this.Method, this.ResourcePath); }
8376
}
8477

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

106-
Uri fullUri;
10799
string relativeUrlString = string.Join("/", urlParts);
108100
Uri relativeUri = new Uri(relativeUrlString, UriKind.Relative);
109-
bool uriCreateSucceeded = Uri.TryCreate(baseUri, relativeUri, out fullUri);
110-
if (!uriCreateSucceeded)
101+
if (!Uri.TryCreate(baseUri, relativeUri, out Uri? fullUri))
111102
{
112-
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));
103+
throw new InvalidOperationException(string.Format(CultureInfo.InvariantCulture, "Unable to create URI from base {0} and relative path {1}", baseUri?.ToString(), relativeUrlString));
113104
}
114105

115106
return fullUri;
@@ -133,11 +124,11 @@ private static string GetCommandPropertyValue(string propertyName, Command comma
133124
{
134125
// Extract the URL parameter, and remove it from the parameters dictionary
135126
// so it doesn't get transmitted as a JSON parameter.
136-
if (commandToExecute.Parameters.ContainsKey(propertyName))
127+
if (commandToExecute.Parameters.TryGetValue(propertyName, out var propertyValueObject))
137128
{
138-
if (commandToExecute.Parameters[propertyName] != null)
129+
if (propertyValueObject != null)
139130
{
140-
propertyValue = commandToExecute.Parameters[propertyName].ToString();
131+
propertyValue = propertyValueObject.ToString()!;
141132
commandToExecute.Parameters.Remove(propertyName);
142133
}
143134
}

dotnet/src/webdriver/ICommandExecutor.cs

+7-2
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,11 @@
1818
// </copyright>
1919

2020
using System;
21+
using System.Diagnostics.CodeAnalysis;
2122
using System.Threading.Tasks;
2223

24+
#nullable enable
25+
2326
namespace OpenQA.Selenium
2427
{
2528
/// <summary>
@@ -31,15 +34,16 @@ public interface ICommandExecutor : IDisposable
3134
/// Attempts to add a command to the repository of commands known to this executor.
3235
/// </summary>
3336
/// <param name="commandName">The name of the command to attempt to add.</param>
34-
/// <param name="info">The <see cref="CommandInfo"/> describing the commnd to add.</param>
37+
/// <param name="info">The <see cref="CommandInfo"/> describing the command to add.</param>
3538
/// <returns><see langword="true"/> if the new command has been added successfully; otherwise, <see langword="false"/>.</returns>
36-
bool TryAddCommand(string commandName, CommandInfo info);
39+
bool TryAddCommand(string commandName, [NotNullWhen(true)] CommandInfo? info);
3740

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

4549

@@ -48,6 +52,7 @@ public interface ICommandExecutor : IDisposable
4852
/// </summary>
4953
/// <param name="commandToExecute">The command you wish to execute</param>
5054
/// <returns>A task object representing the asynchronous operation</returns>
55+
/// <exception cref="ArgumentNullException">If <paramref name="commandToExecute"/> is <see langword="null"/>.</exception>
5156
Task<Response> ExecuteAsync(Command commandToExecute);
5257
}
5358
}

dotnet/src/webdriver/ICustomDriverCommandExecutor.cs

+6-2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@
1818
// </copyright>
1919

2020
using System.Collections.Generic;
21+
using System.Diagnostics.CodeAnalysis;
22+
23+
#nullable enable
2124

2225
namespace OpenQA.Selenium
2326
{
@@ -32,7 +35,8 @@ public interface ICustomDriverCommandExecutor
3235
/// <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>
3336
/// <param name="parameters">A <see cref="Dictionary{K, V}"/> containing the names and values of the parameters of the command.</param>
3437
/// <returns>An object that contains the value returned by the command, if any.</returns>
35-
object ExecuteCustomDriverCommand(string driverCommandToExecute, Dictionary<string, object> parameters);
38+
/// <exception cref="WebDriverException">The command returned an exceptional value.</exception>
39+
object? ExecuteCustomDriverCommand(string driverCommandToExecute, Dictionary<string, object> parameters);
3640

3741
/// <summary>
3842
/// Registers a set of commands to be executed with this driver instance.
@@ -46,6 +50,6 @@ public interface ICustomDriverCommandExecutor
4650
/// <param name="commandName">The unique name of the command to register.</param>
4751
/// <param name="commandInfo">The <see cref="CommandInfo"/> object describing the command.</param>
4852
/// <returns><see langword="true"/> if the command was registered; otherwise, <see langword="false"/>.</returns>
49-
bool RegisterCustomDriverCommand(string commandName, CommandInfo commandInfo);
53+
bool RegisterCustomDriverCommand(string commandName, [NotNullWhen(true)] CommandInfo? commandInfo);
5054
}
5155
}

dotnet/src/webdriver/Remote/DriverServiceCommandExecutor.cs

+4-1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
// </copyright>
1919

2020
using System;
21+
using System.Diagnostics.CodeAnalysis;
2122
using System.Threading.Tasks;
2223

2324
#nullable enable
@@ -78,7 +79,7 @@ public DriverServiceCommandExecutor(DriverService service, HttpCommandExecutor c
7879
// get { return this.HttpExecutor.CommandInfoRepository; }
7980
//}
8081

81-
public bool TryAddCommand(string commandName, CommandInfo info)
82+
public bool TryAddCommand(string commandName, [NotNullWhen(true)] CommandInfo? info)
8283
{
8384
return this.HttpExecutor.TryAddCommand(commandName, info);
8485
}
@@ -94,6 +95,7 @@ public bool TryAddCommand(string commandName, CommandInfo info)
9495
/// </summary>
9596
/// <param name="commandToExecute">The command you wish to execute</param>
9697
/// <returns>A response from the browser</returns>
98+
/// <exception cref="ArgumentNullException">If <paramref name="commandToExecute"/> is <see langword="null"/>.</exception>
9799
public Response Execute(Command commandToExecute)
98100
{
99101
return Task.Run(() => this.ExecuteAsync(commandToExecute)).GetAwaiter().GetResult();
@@ -104,6 +106,7 @@ public Response Execute(Command commandToExecute)
104106
/// </summary>
105107
/// <param name="commandToExecute">The command you wish to execute</param>
106108
/// <returns>A task object representing the asynchronous operation</returns>
109+
/// <exception cref="ArgumentNullException">If <paramref name="commandToExecute"/> is <see langword="null"/>.</exception>
107110
public async Task<Response> ExecuteAsync(Command commandToExecute)
108111
{
109112
if (commandToExecute == null)

0 commit comments

Comments
 (0)