Skip to content

Enable Nullable Reference Types #92

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Enable Nullable Reference Types #92

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f3c3e29
Enable nullable type references
gleono Sep 25, 2024
cd42126
Improve setting env variables on init
gleono Sep 28, 2024
825e323
Merge branch 'apache:master' into nullable-types
gleono Sep 30, 2024
08316bc
Suggestion
gleono Oct 4, 2024
745eecb
Small new features of C#
gleono Oct 7, 2024
695e15b
Updated comment and variable names
gleono Oct 10, 2024
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
1 change: 1 addition & 0 deletions core/net6.0/proxy/Apache.OpenWhisk.Runtime.Common/Apache.OpenWhisk.Runtime.Common.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

<PropertyGroup>
<TargetFramework>net6.0</TargetFramework>
<Nullable>enable</Nullable>
</PropertyGroup>

<ItemGroup>
Expand Down
5 changes: 2 additions & 3 deletions core/net6.0/proxy/Apache.OpenWhisk.Runtime.Common/HttpResponseExtension.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,14 @@ public static class HttpResponseExtension
{
public static async Task WriteResponse(this HttpResponse response, int code, string content)
{
byte[] bytes = Encoding.UTF8.GetBytes(content);
response.ContentLength = bytes.Length;
response.ContentLength = Encoding.UTF8.GetByteCount(content);
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)}};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shorter syntax for constructors. Type is already in the declaration of the variable so, no need to be redundant.

await WriteResponse(response, 502, JsonConvert.SerializeObject(message));
}

Expand Down
94 changes: 43 additions & 51 deletions core/net6.0/proxy/Apache.OpenWhisk.Runtime.Common/Init.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,31 +21,30 @@
using System.Reflection;
using System.Threading;
using System.Threading.Tasks;
using System.Collections.Generic;
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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shorter syntax and clearer parameters.

private Type? _type;
private MethodInfo? _method;
private ConstructorInfo? _constructor;
private bool _awaitableMethod;

public bool Initialized { get; private set; }
private Type Type { get; set; }
private MethodInfo Method { get; set; }
private ConstructorInfo Constructor { get; set; }
private bool AwaitableMethod { get; set; }

public Init()
{
Initialized = false;
Type = null;
Method = null;
Constructor = null;
_type = null;
_method = null;
_constructor = null;
}

public async Task<Run> HandleRequest(HttpContext httpContext)
public async Task<Run?> HandleRequest(HttpContext httpContext)
{
await _initSemaphoreSlim.WaitAsync();
try
Expand All @@ -54,58 +53,52 @@ public async Task<Run> HandleRequest(HttpContext httpContext)
{
await httpContext.Response.WriteError("Cannot initialize the action more than once.");
Console.Error.WriteLine("Cannot initialize the action more than once.");
return (new Run(Type, Method, Constructor, AwaitableMethod));
return new Run(_type, _method, _constructor, _awaitableMethod);
}

string body = await new StreamReader(httpContext.Request.Body).ReadToEndAsync();
using StreamReader reader = new(httpContext.Request.Body);
string body = await reader.ReadToEndAsync();
JObject inputObject = JObject.Parse(body);
if (!inputObject.ContainsKey("value"))
if (!inputObject.TryGetValue("value", out JToken? message) || message is not JObject valueObj)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use pattern matching to assign valueObj right away.

{
await httpContext.Response.WriteError("Missing main/no code to execute.");
return (null);
return null;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why all return values are between parentheses. Is this enforced by the Scalariform plugin?

}

JToken message = inputObject["value"];
string main = valueObj.TryGetValue("main", out JToken? mainToken) ? mainToken.ToString() : string.Empty;
string code = valueObj.TryGetValue("code", out JToken? codeToken) ? codeToken.ToString() : string.Empty;
bool binary = valueObj.TryGetValue("binary", out JToken? binaryToken) && binaryToken.ToObject<bool>();

if (message["main"] == null || message["binary"] == null || message["code"] == null)
if (string.IsNullOrWhiteSpace(main) || string.IsNullOrWhiteSpace(code))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are checking only for white space because main and code are guaranteed to not be null.

{
await httpContext.Response.WriteError("Missing main/no code to execute.");
return (null);
return null;
}

string main = message["main"].ToString();

bool binary = message["binary"].ToObject<bool>();

if (!binary)
{
await httpContext.Response.WriteError("code must be binary (zip file).");
return (null);
return null;
}

string[] mainParts = main.Split("::");
if (mainParts.Length != 3)
{
await httpContext.Response.WriteError("main required format is \"Assembly::Type::Function\".");
return (null);
return null;
}

string tempPath = Path.Combine(Environment.CurrentDirectory, Guid.NewGuid().ToString());
string base64Zip = message["code"].ToString();
try
{
using (MemoryStream stream = new MemoryStream(Convert.FromBase64String(base64Zip)))
{
using (ZipArchive archive = new ZipArchive(stream))
{
archive.ExtractToDirectory(tempPath);
}
}
using MemoryStream stream = new(Convert.FromBase64String(code));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to nest usings anymore.

using ZipArchive archive = new(stream);
archive.ExtractToDirectory(tempPath);
}
catch (Exception)
{
await httpContext.Response.WriteError("Unable to decompress package.");
return (null);
return null;
}

Environment.CurrentDirectory = tempPath;
Expand All @@ -117,31 +110,30 @@ public async Task<Run> HandleRequest(HttpContext httpContext)
if (!File.Exists(assemblyPath))
{
await httpContext.Response.WriteError($"Unable to locate requested assembly (\"{assemblyFile}\").");
return (null);
return null;
}

try
{
// Export init arguments as environment variables
if (message["env"] != null && message["env"].HasValues)
if (valueObj.TryGetValue("env", out JToken? values) && values is JObject dictEnv)
{
Dictionary<string, string> dictEnv = message["env"].ToObject<Dictionary<string, string>>();
foreach (KeyValuePair<string, string> entry in dictEnv) {
foreach ((string envKey, JToken? envVal) in dictEnv) {
// See https://docs.microsoft.com/en-us/dotnet/api/system.environment.setenvironmentvariable
// If entry.Value is null or the empty string, the variable is not set
Environment.SetEnvironmentVariable(entry.Key, entry.Value);
// If envVal is null or the empty string, the variable is not set
Environment.SetEnvironmentVariable(envKey, envVal?.ToString());
}
}

Assembly assembly = Assembly.LoadFrom(assemblyPath);
Type = assembly.GetType(mainParts[1]);
if (Type == null)
_type = assembly.GetType(mainParts[1]);
if (_type == null)
{
await httpContext.Response.WriteError($"Unable to locate requested type (\"{mainParts[1]}\").");
return (null);
return null;
}
Method = Type.GetMethod(mainParts[2]);
Constructor = Type.GetConstructor(Type.EmptyTypes);
_method = _type.GetMethod(mainParts[2]);
_constructor = _type.GetConstructor(Type.EmptyTypes);
}
catch (Exception ex)
{
Expand All @@ -151,26 +143,26 @@ await httpContext.Response.WriteError(ex.Message
+ ", " + ex.StackTrace
#endif
);
return (null);
return null;
}

if (Method == null)
if (_method == null)
{
await httpContext.Response.WriteError($"Unable to locate requested method (\"{mainParts[2]}\").");
return (null);
return null;
}

if (Constructor == null)
if (_constructor == null)
{
await httpContext.Response.WriteError($"Unable to locate appropriate constructor for (\"{mainParts[1]}\").");
return (null);
return null;
}

Initialized = true;

AwaitableMethod = (Method.ReturnType.GetMethod(nameof(Task.GetAwaiter)) != null);
_awaitableMethod = _method.ReturnType.GetMethod(nameof(Task.GetAwaiter)) != null;

return (new Run(Type, Method, Constructor, AwaitableMethod));
return new Run(_type, _method, _constructor, _awaitableMethod);
}
catch (Exception ex)
{
Expand All @@ -181,7 +173,7 @@ await httpContext.Response.WriteError(ex.Message
#endif
);
Startup.WriteLogMarkers();
return (null);
return null;
}
finally
{
Expand Down
31 changes: 16 additions & 15 deletions core/net6.0/proxy/Apache.OpenWhisk.Runtime.Common/Run.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ namespace Apache.OpenWhisk.Runtime.Common
{
public class Run
{
private readonly Type _type;
private readonly MethodInfo _method;
private readonly ConstructorInfo _constructor;
private readonly Type? _type;
private readonly MethodInfo? _method;
private readonly ConstructorInfo? _constructor;
private readonly bool _awaitableMethod;

public Run(Type type, MethodInfo method, ConstructorInfo constructor, bool awaitableMethod)
public Run(Type? type, MethodInfo? method, ConstructorInfo? constructor, bool awaitableMethod)
{
_type = type;
_method = method;
Expand All @@ -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);
Copy link
Contributor Author

@gleono gleono Oct 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's be good citizens and dispose any unmanaged resources created by the StreamReader.

string body = await reader.ReadToEndAsync();

JObject inputObject = string.IsNullOrEmpty(body) ? null : JObject.Parse(body);
JObject? inputObject = string.IsNullOrEmpty(body) ? null : JObject.Parse(body);

JObject valObject = null;
JArray valArray = null;
JObject? valObject = null;
JArray? valArray = null;

if (inputObject != null)
{
Expand All @@ -66,7 +67,7 @@ public async Task HandleRequest(HttpContext httpContext)
if (token.Path.Equals("value", StringComparison.InvariantCultureIgnoreCase))
continue;
string envKey = $"__OW_{token.Path.ToUpperInvariant()}";
string envVal = token.First.ToString();
string? envVal = token.First?.ToString();
Environment.SetEnvironmentVariable(envKey, envVal);
//Console.WriteLine($"Set environment variable \"{envKey}\" to \"{envVal}\".");
}
Expand All @@ -81,24 +82,24 @@ await Console.Error.WriteLineAsync(
}
}

object owObject = _constructor.Invoke(new object[] { });
object owObject = _constructor.Invoke(Array.Empty<object>());
Copy link
Contributor Author

@gleono gleono Oct 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to allocate an object to invoke the constructor.


try
{
JContainer output;
JContainer? output;

if(_awaitableMethod) {
if (valObject != null) {
output = (JContainer) await (dynamic) _method.Invoke(owObject, new object[] {valObject});
output = (JContainer?) await (dynamic?) _method.Invoke(owObject, new object?[] {valObject});
} else {
output = (JContainer) await (dynamic) _method.Invoke(owObject, new object[] {valArray});
output = (JContainer?) await (dynamic?) _method.Invoke(owObject, new object?[] {valArray});
}
}
else {
if (valObject != null) {
output = (JContainer) _method.Invoke(owObject, new object[] {valObject});
output = (JContainer?) _method.Invoke(owObject, new object?[] {valObject});
} else {
output = (JContainer) _method.Invoke(owObject, new object[] {valArray});
output = (JContainer?) _method.Invoke(owObject, new object?[] {valArray});
}
}

Expand Down
8 changes: 4 additions & 4 deletions core/net6.0/proxy/Apache.OpenWhisk.Runtime.Common/Startup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ public static void WriteLogMarkers()

public void Configure(IApplicationBuilder app)
{
PathString initPath = new PathString("/init");
PathString runPath = new PathString("/run");
Init init = new Init();
Run run = null;
PathString initPath = new("/init");
PathString runPath = new("/run");
Init init = new();
Run? run = null;
app.Run(async (httpContext) =>
{
if (httpContext.Request.Path.Equals(initPath))
Expand Down
1 change: 1 addition & 0 deletions ...xy/Apache.OpenWhisk.Runtime.Dotnet.Minimal/Apache.OpenWhisk.Runtime.Dotnet.Minimal.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net6.0</TargetFramework>
<Nullable>enable</Nullable>
</PropertyGroup>

<ItemGroup>
Expand Down