From 326c597b957405d94c5127e75f4c45f03d757c78 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Sat, 30 Nov 2024 23:42:42 -0500 Subject: [PATCH 1/8] [dotnet] Modernize `Response` type --- dotnet/src/webdriver/Response.cs | 82 ++++++++++++------------------- dotnet/src/webdriver/WebDriver.cs | 6 +-- 2 files changed, 33 insertions(+), 55 deletions(-) diff --git a/dotnet/src/webdriver/Response.cs b/dotnet/src/webdriver/Response.cs index 39962a0484a1f..1f825d5faa95e 100644 --- a/dotnet/src/webdriver/Response.cs +++ b/dotnet/src/webdriver/Response.cs @@ -31,16 +31,12 @@ namespace OpenQA.Selenium /// public class Response { - private readonly static JsonSerializerOptions s_jsonSerializerOptions = new() + private static readonly JsonSerializerOptions s_jsonSerializerOptions = new() { TypeInfoResolver = ResponseJsonSerializerContext.Default, Converters = { new ResponseValueJsonConverter() } // we still need it to make `Object` as `Dictionary` }; - private object responseValue; - private string responseSessionId; - private WebDriverResult responseStatus; - /// /// Initializes a new instance of the class /// @@ -56,23 +52,29 @@ public Response(SessionId sessionId) { if (sessionId != null) { - this.responseSessionId = sessionId.ToString(); + this.SessionId = sessionId.ToString(); } } + internal Response(WebDriverResult status, object value) + { + this.Status = status; + this.Value = value; + } + private Response(Dictionary rawResponse) { if (rawResponse.ContainsKey("sessionId")) { if (rawResponse["sessionId"] != null) { - this.responseSessionId = rawResponse["sessionId"].ToString(); + this.SessionId = rawResponse["sessionId"].ToString(); } } - if (rawResponse.ContainsKey("value")) + if (rawResponse.TryGetValue("value", out object value)) { - this.responseValue = rawResponse["value"]; + this.Value = value; } // If the returned object does *not* have a "value" property @@ -80,35 +82,34 @@ private Response(Dictionary rawResponse) // TODO: Remove this if statement altogether; there should // never be a spec-compliant response that does not contain a // value property. - if (!rawResponse.ContainsKey("value") && this.responseValue == null) + if (!rawResponse.ContainsKey("value") && this.Value == null) { // Special-case for the new session command, where the "capabilities" // property of the response is the actual value we're interested in. if (rawResponse.ContainsKey("capabilities")) { - this.responseValue = rawResponse["capabilities"]; + this.Value = rawResponse["capabilities"]; } else { - this.responseValue = rawResponse; + this.Value = rawResponse; } } - Dictionary valueDictionary = this.responseValue as Dictionary; - if (valueDictionary != null) + if (this.Value is Dictionary valueDictionary) { // Special case code for the new session command. If the response contains // sessionId and capabilities properties, fix up the session ID and value members. if (valueDictionary.ContainsKey("sessionId")) { - this.responseSessionId = valueDictionary["sessionId"].ToString(); - if (valueDictionary.ContainsKey("capabilities")) + this.SessionId = valueDictionary["sessionId"].ToString(); + if (valueDictionary.TryGetValue("capabilities", out object capabilities)) { - this.responseValue = valueDictionary["capabilities"]; + this.Value = capabilities; } else { - this.responseValue = valueDictionary["value"]; + this.Value = valueDictionary["value"]; } } } @@ -117,29 +118,17 @@ private Response(Dictionary rawResponse) /// /// Gets or sets the value from JSON. /// - public object Value - { - get { return this.responseValue; } - set { this.responseValue = value; } - } + public object Value { get; set; } /// /// Gets or sets the session ID. /// - public string SessionId - { - get { return this.responseSessionId; } - set { this.responseSessionId = value; } - } + public string SessionId { get; set; } /// /// Gets or sets the status value of the response. /// - public WebDriverResult Status - { - get { return this.responseStatus; } - set { this.responseStatus = value; } - } + public WebDriverResult Status { get; set; } /// /// Returns a new from a JSON-encoded string. @@ -148,9 +137,10 @@ public WebDriverResult Status /// A object described by the JSON string. public static Response FromJson(string value) { - Dictionary deserializedResponse = JsonSerializer.Deserialize>(value, s_jsonSerializerOptions); - Response response = new Response(deserializedResponse); - return response; + Dictionary deserializedResponse = JsonSerializer.Deserialize>(value, s_jsonSerializerOptions) + ?? throw new WebDriverException("JSON success response returned \"null\" value"); + + return new Response(deserializedResponse); } /// @@ -160,9 +150,8 @@ public static Response FromJson(string value) /// A object described by the JSON string. public static Response FromErrorJson(string value) { - var deserializedResponse = JsonSerializer.Deserialize>(value, s_jsonSerializerOptions); - - var response = new Response(); + var deserializedResponse = JsonSerializer.Deserialize>(value, s_jsonSerializerOptions) + ?? throw new WebDriverException("JSON error response returned \"null\" value"); if (!deserializedResponse.TryGetValue("value", out var valueObject)) { @@ -174,23 +163,19 @@ public static Response FromErrorJson(string value) throw new WebDriverException($"The 'value' property is not a dictionary of {Environment.NewLine}{value}"); } - response.Value = valueDictionary; - if (!valueDictionary.TryGetValue("error", out var errorObject)) { throw new WebDriverException($"The 'value > error' property was not found in the response:{Environment.NewLine}{value}"); } - if (errorObject is not string) + if (errorObject is not string errorString) { throw new WebDriverException($"The 'value > error' property is not a string{Environment.NewLine}{value}"); } - response.Value = deserializedResponse["value"]; + var status = WebDriverError.ResultFromError(errorString); - response.Status = WebDriverError.ResultFromError(errorObject.ToString()); - - return response; + return new Response(status, valueDictionary); } /// @@ -213,8 +198,5 @@ public override string ToString() } [JsonSerializable(typeof(Dictionary))] - internal partial class ResponseJsonSerializerContext : JsonSerializerContext - { - - } + internal sealed partial class ResponseJsonSerializerContext : JsonSerializerContext; } diff --git a/dotnet/src/webdriver/WebDriver.cs b/dotnet/src/webdriver/WebDriver.cs index d9960ea97b235..77eda58b5dd22 100644 --- a/dotnet/src/webdriver/WebDriver.cs +++ b/dotnet/src/webdriver/WebDriver.cs @@ -622,11 +622,7 @@ protected virtual async Task ExecuteAsync(string driverCommandToExecut } catch (System.Net.Http.HttpRequestException e) { - commandResponse = new Response - { - Status = WebDriverResult.UnhandledError, - Value = e - }; + commandResponse = new Response(WebDriverResult.UnhandledError, e); } if (commandResponse.Status != WebDriverResult.Success) From 80231f67c33595a0970a36d31274163c8806c980 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Sat, 30 Nov 2024 23:58:26 -0500 Subject: [PATCH 2/8] Add `Response` ctor that includes session ID --- dotnet/src/webdriver/Response.cs | 5 +++-- dotnet/src/webdriver/WebDriver.cs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/dotnet/src/webdriver/Response.cs b/dotnet/src/webdriver/Response.cs index 1f825d5faa95e..cc21fcd8ee006 100644 --- a/dotnet/src/webdriver/Response.cs +++ b/dotnet/src/webdriver/Response.cs @@ -56,8 +56,9 @@ public Response(SessionId sessionId) } } - internal Response(WebDriverResult status, object value) + internal Response(SessionId sessionId, WebDriverResult status, object value) { + this.SessionId = sessionId?.ToString(); this.Status = status; this.Value = value; } @@ -175,7 +176,7 @@ public static Response FromErrorJson(string value) var status = WebDriverError.ResultFromError(errorString); - return new Response(status, valueDictionary); + return new Response(sessionId: null, status, valueDictionary); } /// diff --git a/dotnet/src/webdriver/WebDriver.cs b/dotnet/src/webdriver/WebDriver.cs index 77eda58b5dd22..897277ce132bf 100644 --- a/dotnet/src/webdriver/WebDriver.cs +++ b/dotnet/src/webdriver/WebDriver.cs @@ -622,7 +622,7 @@ protected virtual async Task ExecuteAsync(string driverCommandToExecut } catch (System.Net.Http.HttpRequestException e) { - commandResponse = new Response(WebDriverResult.UnhandledError, e); + commandResponse = new Response(sessionId: null, WebDriverResult.UnhandledError, e); } if (commandResponse.Status != WebDriverResult.Success) From e76f54068be9d6d9e4230c7dfa696b83d0e256e3 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Sun, 8 Dec 2024 00:27:58 -0500 Subject: [PATCH 3/8] undo `WebDriver` changes --- dotnet/src/webdriver/WebDriver.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dotnet/src/webdriver/WebDriver.cs b/dotnet/src/webdriver/WebDriver.cs index b5c6d3f0e2978..9bcca8b28f8b3 100644 --- a/dotnet/src/webdriver/WebDriver.cs +++ b/dotnet/src/webdriver/WebDriver.cs @@ -622,7 +622,11 @@ protected virtual async Task ExecuteAsync(string driverCommandToExecut } catch (System.Net.Http.HttpRequestException e) { - commandResponse = new Response(sessionId: null, WebDriverResult.UnhandledError, e); + commandResponse = new Response + { + Status = WebDriverResult.UnhandledError, + Value = e + }; } if (commandResponse.Status != WebDriverResult.Success) From 429fa8dd432c6f5f1021e563ef002abb33ab930f Mon Sep 17 00:00:00 2001 From: Michael Render Date: Sun, 8 Dec 2024 00:29:40 -0500 Subject: [PATCH 4/8] Remove unnecessary diffs from `Response.FromErrorJson` --- dotnet/src/webdriver/Response.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/dotnet/src/webdriver/Response.cs b/dotnet/src/webdriver/Response.cs index cc21fcd8ee006..b78eb0c770be6 100644 --- a/dotnet/src/webdriver/Response.cs +++ b/dotnet/src/webdriver/Response.cs @@ -174,9 +174,12 @@ public static Response FromErrorJson(string value) throw new WebDriverException($"The 'value > error' property is not a string{Environment.NewLine}{value}"); } - var status = WebDriverError.ResultFromError(errorString); + var response = new Response(); - return new Response(sessionId: null, status, valueDictionary); + response.Value = valueDictionary; + response.Status = WebDriverError.ResultFromError(errorString); + + return response; } /// From bdb61135fd3b73fa00fa9bd06719bbb18077ce63 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Sun, 8 Dec 2024 00:30:27 -0500 Subject: [PATCH 5/8] minimize diffs further --- dotnet/src/webdriver/Response.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dotnet/src/webdriver/Response.cs b/dotnet/src/webdriver/Response.cs index b78eb0c770be6..c27f4146ad31a 100644 --- a/dotnet/src/webdriver/Response.cs +++ b/dotnet/src/webdriver/Response.cs @@ -151,6 +151,8 @@ public static Response FromJson(string value) /// A object described by the JSON string. public static Response FromErrorJson(string value) { + var response = new Response(); + var deserializedResponse = JsonSerializer.Deserialize>(value, s_jsonSerializerOptions) ?? throw new WebDriverException("JSON error response returned \"null\" value"); @@ -174,8 +176,6 @@ public static Response FromErrorJson(string value) throw new WebDriverException($"The 'value > error' property is not a string{Environment.NewLine}{value}"); } - var response = new Response(); - response.Value = valueDictionary; response.Status = WebDriverError.ResultFromError(errorString); From 8d53a1d1604728089f1d2e27773d0bd8bfa56bea Mon Sep 17 00:00:00 2001 From: Michael Render Date: Sun, 8 Dec 2024 00:31:31 -0500 Subject: [PATCH 6/8] further whitespace diff minimizing --- dotnet/src/webdriver/Response.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/dotnet/src/webdriver/Response.cs b/dotnet/src/webdriver/Response.cs index c27f4146ad31a..1554c3ee67531 100644 --- a/dotnet/src/webdriver/Response.cs +++ b/dotnet/src/webdriver/Response.cs @@ -151,11 +151,11 @@ public static Response FromJson(string value) /// A object described by the JSON string. public static Response FromErrorJson(string value) { - var response = new Response(); - var deserializedResponse = JsonSerializer.Deserialize>(value, s_jsonSerializerOptions) ?? throw new WebDriverException("JSON error response returned \"null\" value"); + var response = new Response(); + if (!deserializedResponse.TryGetValue("value", out var valueObject)) { throw new WebDriverException($"The 'value' property was not found in the response:{Environment.NewLine}{value}"); @@ -166,6 +166,8 @@ public static Response FromErrorJson(string value) throw new WebDriverException($"The 'value' property is not a dictionary of {Environment.NewLine}{value}"); } + response.Value = valueDictionary; + if (!valueDictionary.TryGetValue("error", out var errorObject)) { throw new WebDriverException($"The 'value > error' property was not found in the response:{Environment.NewLine}{value}"); @@ -176,7 +178,6 @@ public static Response FromErrorJson(string value) throw new WebDriverException($"The 'value > error' property is not a string{Environment.NewLine}{value}"); } - response.Value = valueDictionary; response.Status = WebDriverError.ResultFromError(errorString); return response; From 63c2478ac82ee6d119c2fe233fc18fc5e7fdb553 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Sun, 8 Dec 2024 00:32:40 -0500 Subject: [PATCH 7/8] one less diff --- dotnet/src/webdriver/Response.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dotnet/src/webdriver/Response.cs b/dotnet/src/webdriver/Response.cs index 1554c3ee67531..acbb8d5b0962d 100644 --- a/dotnet/src/webdriver/Response.cs +++ b/dotnet/src/webdriver/Response.cs @@ -178,6 +178,8 @@ public static Response FromErrorJson(string value) throw new WebDriverException($"The 'value > error' property is not a string{Environment.NewLine}{value}"); } + response.Value = deserializedResponse["value"]; + response.Status = WebDriverError.ResultFromError(errorString); return response; From 86ec56838f682bb4d67fedcd381de46d4abfe979 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Sun, 8 Dec 2024 00:34:54 -0500 Subject: [PATCH 8/8] remove unused interal ctor from `Response` --- dotnet/src/webdriver/Response.cs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/dotnet/src/webdriver/Response.cs b/dotnet/src/webdriver/Response.cs index acbb8d5b0962d..0af9b181779b3 100644 --- a/dotnet/src/webdriver/Response.cs +++ b/dotnet/src/webdriver/Response.cs @@ -56,13 +56,6 @@ public Response(SessionId sessionId) } } - internal Response(SessionId sessionId, WebDriverResult status, object value) - { - this.SessionId = sessionId?.ToString(); - this.Status = status; - this.Value = value; - } - private Response(Dictionary rawResponse) { if (rawResponse.ContainsKey("sessionId"))