Skip to content

[dotnet] Move Response constructors towards immutability #14998

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jan 3, 2025
Merged

[dotnet] Move Response constructors towards immutability #14998

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3331a86
[dotnet] Move `Response` constructors towards immutability
RenderMichael Jan 1, 2025
6325827
Add nullable reference type annotations to `Response`
RenderMichael Jan 1, 2025
eb4341f
fix nullability for `as` casts
RenderMichael Jan 1, 2025
6285202
Merge branch 'trunk' into response-type
RenderMichael Jan 2, 2025
d9f8c4c
Merge branch 'trunk' into response-type
nvborisenko Jan 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions dotnet/src/webdriver/Remote/HttpCommandExecutor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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))
Expand All @@ -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;
Expand Down
72 changes: 37 additions & 35 deletions dotnet/src/webdriver/Response.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand Down Expand Up @@ -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}");
Expand All @@ -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}");
Expand All @@ -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>
Expand Down