Enable Nullable Reference Types #92
Conversation
core/net6.0/proxy/Apache.OpenWhisk.Runtime.Common/HttpResponseExtension.cs
Show resolved
Hide resolved
| response.StatusCode = code; | ||
| await response.WriteAsync(content); | ||
| } | ||
|
|
||
| public static async Task WriteError(this HttpResponse response, string errorMessage) | ||
| { | ||
| JObject message = new JObject {{"error", new JValue(errorMessage)}}; | ||
| JObject message = new() {{"error", new JValue(errorMessage)}}; |
There was a problem hiding this comment.
Shorter syntax for constructors. Type is already in the declaration of the variable so, no need to be redundant.
| using Microsoft.AspNetCore.Http; | ||
| using Newtonsoft.Json.Linq; | ||
|
|
||
| namespace Apache.OpenWhisk.Runtime.Common | ||
| { | ||
| public class Init | ||
| { | ||
| private readonly SemaphoreSlim _initSemaphoreSlim = new SemaphoreSlim(1, 1); | ||
| private readonly SemaphoreSlim _initSemaphoreSlim = new(initialCount: 1, maxCount: 1); |
There was a problem hiding this comment.
Shorter syntax and clearer parameters.
| JObject inputObject = JObject.Parse(body); | ||
| if (!inputObject.ContainsKey("value")) | ||
| if (!inputObject.TryGetValue("value", out JToken? message) || message is not JObject valueObj) |
There was a problem hiding this comment.
Use pattern matching to assign valueObj right away.
| { | ||
| await httpContext.Response.WriteError("Missing main/no code to execute."); | ||
| return (null); | ||
| return null; |
There was a problem hiding this comment.
I am not sure why all return values are between parentheses. Is this enforced by the Scalariform plugin?
|
|
||
| if (message["main"] == null || message["binary"] == null || message["code"] == null) | ||
| if (string.IsNullOrWhiteSpace(main) || string.IsNullOrWhiteSpace(code)) |
There was a problem hiding this comment.
These are checking only for white space because main and code are guaranteed to not be null.
| archive.ExtractToDirectory(tempPath); | ||
| } | ||
| } | ||
| using MemoryStream stream = new(Convert.FromBase64String(code)); |
There was a problem hiding this comment.
No need to nest usings anymore.
| @@ -49,12 +49,13 @@ public async Task HandleRequest(HttpContext httpContext) | |||
|
|
|||
| try | |||
| { | |||
| string body = await new StreamReader(httpContext.Request.Body).ReadToEndAsync(); | |||
| using StreamReader reader = new(httpContext.Request.Body); | |||
There was a problem hiding this comment.
Let's be good citizens and dispose any unmanaged resources created by the StreamReader.
| @@ -81,24 +82,24 @@ await Console.Error.WriteLineAsync( | |||
| } | |||
| } | |||
|
|
|||
| object owObject = _constructor.Invoke(new object[] { }); | |||
| object owObject = _constructor.Invoke(Array.Empty<object>()); | |||
There was a problem hiding this comment.
No need to allocate an object to invoke the constructor.
|
I forgot to mention that I still cannot run the tests locally in my machine since I can only run openwhisk in standalone mode in a docker container and I don't know how to run the tests against it. I tested manually by running the container for this runtime and sending the base64 encoded tests to it in the expected JSON text. It seems to work fine that way. I found some issues while trying to test it in integration with openwhisk in standalone mode. Specially the array input and output cases. |
I mentioned that this could be a good improvement on the code for this runtime in #91
I probably got carried away with some other changes not specific to enabling NRTs. I will explain my rationale for these and ask questions about other minor details in the code.