From 3331a86a93e37ccd8b19dd3114e218e428b2e07a Mon Sep 17 00:00:00 2001 From: Michael Render Date: Wed, 1 Jan 2025 00:58:25 -0500 Subject: [PATCH 1/3] [dotnet] Move `Response` constructors towards immutability --- .../webdriver/Remote/HttpCommandExecutor.cs | 11 ++- dotnet/src/webdriver/Response.cs | 72 ++++++++++--------- 2 files changed, 42 insertions(+), 41 deletions(-) diff --git a/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs b/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs index 0686239405e38..85921d73954be 100644 --- a/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs +++ b/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs @@ -316,7 +316,7 @@ private async Task MakeHttpRequest(HttpRequestInfo requestInfo private Response CreateResponse(HttpResponseInfo responseInfo) { - Response response = new Response(); + Response response; string body = responseInfo.Body; if ((int)responseInfo.StatusCode < 200 || (int)responseInfo.StatusCode > 299) { @@ -326,8 +326,7 @@ private Response CreateResponse(HttpResponseInfo responseInfo) } else { - response.Status = WebDriverResult.UnknownError; - response.Value = body; + response = new Response(sessionId: null, body, WebDriverResult.UnknownError); } } else if (responseInfo.ContentType != null && responseInfo.ContentType.StartsWith(JsonMimeType, StringComparison.OrdinalIgnoreCase)) @@ -336,12 +335,12 @@ private Response CreateResponse(HttpResponseInfo responseInfo) } else { - response.Value = body; + response = new Response(sessionId: null, body, WebDriverResult.Success); } - if (response.Value is string) + if (response.Value is string valueString) { - response.Value = ((string)response.Value).Replace("\r\n", "\n").Replace("\n", Environment.NewLine); + response.Value = valueString.Replace("\r\n", "\n").Replace("\n", Environment.NewLine); } return response; diff --git a/dotnet/src/webdriver/Response.cs b/dotnet/src/webdriver/Response.cs index abe4bb2a9fcdd..9f9a3e1b4f7ea 100644 --- a/dotnet/src/webdriver/Response.cs +++ b/dotnet/src/webdriver/Response.cs @@ -50,10 +50,20 @@ public Response() /// Session ID in use public Response(SessionId sessionId) { - if (sessionId != null) - { - this.SessionId = sessionId.ToString(); - } + this.SessionId = sessionId?.ToString(); + } + + /// + /// Initializes a new instance of the class + /// + /// The Session ID in use, if any. + /// The JSON payload of the response. + /// The WebDriver result status of the response. + public Response(string sessionId, object value, WebDriverResult status) + { + this.SessionId = sessionId; + this.Value = value; + this.Status = status; } /// @@ -66,59 +76,57 @@ public static Response FromJson(string value) Dictionary rawResponse = JsonSerializer.Deserialize>(value, s_jsonSerializerOptions) ?? throw new WebDriverException("JSON success response returned \"null\" value"); - var response = new Response(); + object contents; + string sessionId = null; - if (rawResponse.ContainsKey("sessionId")) + if (rawResponse.TryGetValue("sessionId", out var s) && s is not null) { - if (rawResponse["sessionId"] != null) - { - response.SessionId = rawResponse["sessionId"].ToString(); - } + sessionId = s.ToString(); } if (rawResponse.TryGetValue("value", out object valueObj)) { - response.Value = valueObj; + contents = valueObj; } - - // If the returned object does *not* have a "value" property - // the response value should be the entirety of the response. - // 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") && response.Value == null) + else { + // If the returned object does *not* have a "value" property + // the response value should be the entirety of the response. + // TODO: Remove this if statement altogether; there should + // never be a spec-compliant response that does not contain a + // value property. + // 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")) + if (rawResponse.TryGetValue("capabilities", out var capabilities)) { - response.Value = rawResponse["capabilities"]; + contents = capabilities; } else { - response.Value = rawResponse; + contents = rawResponse; } } - if (response.Value is Dictionary valueDictionary) + if (contents 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")) + if (valueDictionary.TryGetValue("sessionId", out var session)) { - response.SessionId = valueDictionary["sessionId"].ToString(); + sessionId = session.ToString(); if (valueDictionary.TryGetValue("capabilities", out object capabilities)) { - response.Value = capabilities; + contents = capabilities; } else { - response.Value = valueDictionary["value"]; + contents = valueDictionary["value"]; } } } - return response; + return new Response(sessionId, contents, WebDriverResult.Success); } /// @@ -147,8 +155,6 @@ public static Response FromErrorJson(string value) 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}"); @@ -159,8 +165,6 @@ 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}"); @@ -171,11 +175,9 @@ 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); + WebDriverResult status = WebDriverError.ResultFromError(errorString); - return response; + return new Response(sessionId: null, valueDictionary, status); } /// From 63258270b29a7394ab03656570129ea83bbbef21 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Wed, 1 Jan 2025 01:14:22 -0500 Subject: [PATCH 2/3] Add nullable reference type annotations to `Response` --- dotnet/src/webdriver/Response.cs | 37 +++++++++++++++++++------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/dotnet/src/webdriver/Response.cs b/dotnet/src/webdriver/Response.cs index 9f9a3e1b4f7ea..572103b265108 100644 --- a/dotnet/src/webdriver/Response.cs +++ b/dotnet/src/webdriver/Response.cs @@ -24,6 +24,8 @@ using System.Text.Json; using System.Text.Json.Serialization; +#nullable enable + namespace OpenQA.Selenium { /// @@ -48,7 +50,7 @@ public Response() /// Initializes a new instance of the class /// /// Session ID in use - public Response(SessionId sessionId) + public Response(SessionId? sessionId) { this.SessionId = sessionId?.ToString(); } @@ -59,7 +61,7 @@ public Response(SessionId sessionId) /// The Session ID in use, if any. /// The JSON payload of the response. /// The WebDriver result status of the response. - public Response(string sessionId, object value, WebDriverResult status) + public Response(string? sessionId, object? value, WebDriverResult status) { this.SessionId = sessionId; this.Value = value; @@ -71,20 +73,22 @@ public Response(string sessionId, object value, WebDriverResult status) /// /// The JSON string to deserialize into a . /// A object described by the JSON string. + /// If is . + /// If is not a valid JSON object. public static Response FromJson(string value) { - Dictionary rawResponse = JsonSerializer.Deserialize>(value, s_jsonSerializerOptions) + Dictionary rawResponse = JsonSerializer.Deserialize>(value, s_jsonSerializerOptions) ?? throw new WebDriverException("JSON success response returned \"null\" value"); - object contents; - string sessionId = null; + object? contents; + string? sessionId = null; - if (rawResponse.TryGetValue("sessionId", out var s) && s is not null) + if (rawResponse.TryGetValue("sessionId", out object? s) && s is not null) { sessionId = s.ToString(); } - if (rawResponse.TryGetValue("value", out object valueObj)) + if (rawResponse.TryGetValue("value", out object? valueObj)) { contents = valueObj; } @@ -98,7 +102,7 @@ public static Response FromJson(string value) // Special-case for the new session command, where the "capabilities" // property of the response is the actual value we're interested in. - if (rawResponse.TryGetValue("capabilities", out var capabilities)) + if (rawResponse.TryGetValue("capabilities", out object? capabilities)) { contents = capabilities; } @@ -112,10 +116,10 @@ public static Response FromJson(string value) { // 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.TryGetValue("sessionId", out var session)) + if (valueDictionary.TryGetValue("sessionId", out object? session)) { sessionId = session.ToString(); - if (valueDictionary.TryGetValue("capabilities", out object capabilities)) + if (valueDictionary.TryGetValue("capabilities", out object? capabilities)) { contents = capabilities; } @@ -132,12 +136,12 @@ public static Response FromJson(string value) /// /// Gets or sets the value from JSON. /// - public object Value { get; set; } + public object? Value { get; set; } /// /// Gets or sets the session ID. /// - public string SessionId { get; set; } + public string? SessionId { get; set; } /// /// Gets or sets the status value of the response. @@ -150,12 +154,15 @@ public static Response FromJson(string value) /// /// The JSON string to deserialize into a . /// A object described by the JSON string. + /// If is . + /// If is not a valid JSON object. + /// If the JSON dictionary is not in the expected state, per spec. public static Response FromErrorJson(string value) { - var deserializedResponse = JsonSerializer.Deserialize>(value, s_jsonSerializerOptions) + Dictionary deserializedResponse = JsonSerializer.Deserialize>(value, s_jsonSerializerOptions) ?? throw new WebDriverException("JSON error response returned \"null\" value"); - if (!deserializedResponse.TryGetValue("value", out var valueObject)) + if (!deserializedResponse.TryGetValue("value", out object? valueObject)) { throw new WebDriverException($"The 'value' property was not found in the response:{Environment.NewLine}{value}"); } @@ -165,7 +172,7 @@ public static Response FromErrorJson(string value) throw new WebDriverException($"The 'value' property is not a dictionary of {Environment.NewLine}{value}"); } - if (!valueDictionary.TryGetValue("error", out var errorObject)) + if (!valueDictionary.TryGetValue("error", out object? errorObject)) { throw new WebDriverException($"The 'value > error' property was not found in the response:{Environment.NewLine}{value}"); } From eb4341f61c7fece2d4426426557622a47e4b02bf Mon Sep 17 00:00:00 2001 From: Michael Render Date: Wed, 1 Jan 2025 01:15:52 -0500 Subject: [PATCH 3/3] fix nullability for `as` casts --- 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 572103b265108..9a0de908859a5 100644 --- a/dotnet/src/webdriver/Response.cs +++ b/dotnet/src/webdriver/Response.cs @@ -112,7 +112,7 @@ public static Response FromJson(string value) } } - if (contents is Dictionary valueDictionary) + if (contents 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. @@ -167,7 +167,7 @@ public static Response FromErrorJson(string value) throw new WebDriverException($"The 'value' property was not found in the response:{Environment.NewLine}{value}"); } - if (valueObject is not Dictionary valueDictionary) + if (valueObject is not Dictionary valueDictionary) { throw new WebDriverException($"The 'value' property is not a dictionary of {Environment.NewLine}{value}"); }