[dotnet] Move Response constructors towards immutability

Author: RenderMichael

dotnet/src/webdriver/Remote/HttpCommandExecutor.cs

@@ -316,7 +316,7 @@ private async Task<HttpResponseInfo> 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;

dotnet/src/webdriver/Response.cs

@@ -50,10 +50,20 @@ public Response()
         /// <param name="sessionId">Session ID in use</param>
         public Response(SessionId sessionId)
         {
-            if (sessionId != null)
-            {
-                this.SessionId = sessionId.ToString();
-            }
+            this.SessionId = sessionId?.ToString();
+        }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="Response"/> class
+        /// </summary>
+        /// <param name="sessionId">The Session ID in use, if any.</param>
+        /// <param name="value">The JSON payload of the response.</param>
+        /// <param name="status">The WebDriver result status of the response.</param>
+        public Response(string sessionId, object value, WebDriverResult status)
+        {
+            this.SessionId = sessionId;
+            this.Value = value;
+            this.Status = status;
         }
 
         /// <summary>
@@ -66,59 +76,57 @@ public static Response FromJson(string value)
             Dictionary<string, object> rawResponse = JsonSerializer.Deserialize<Dictionary<string, object>>(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<string, object> valueDictionary)
+            if (contents is Dictionary<string, object> 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);
         }
 
         /// <summary>
@@ -147,8 +155,6 @@ public static Response FromErrorJson(string value)
             var deserializedResponse = JsonSerializer.Deserialize<Dictionary<string, object>>(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 <string, object>{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);
         }
 
         /// <summary>