From 060dec440264e5d166cd182ccca83c1d5e2297fd Mon Sep 17 00:00:00 2001 From: Doug Jacob Date: Thu, 6 Feb 2025 13:34:46 -0700 Subject: [PATCH 1/2] Removed unnecessary arg types from EngineWrapper.Work --- .../ClearScriptWrappers/EngineWrapper.cs | 37 +++++-------------- Trell.Test/Engine/EngineTest.cs | 2 +- Trell/Rpc/ToEngine.cs | 10 ++--- 3 files changed, 15 insertions(+), 34 deletions(-) diff --git a/Trell.Engine/ClearScriptWrappers/EngineWrapper.cs b/Trell.Engine/ClearScriptWrappers/EngineWrapper.cs index 3f1591d..911391a 100644 --- a/Trell.Engine/ClearScriptWrappers/EngineWrapper.cs +++ b/Trell.Engine/ClearScriptWrappers/EngineWrapper.cs @@ -17,17 +17,9 @@ public sealed record Work( AbsolutePath SourceDirectory, string Name ) { - public abstract record ArgType { - public sealed record Json(string Name, string JsonString) : ArgType; - public sealed record Raw(string Name, IScriptObject Object) : ArgType; - public sealed record None : ArgType; + public sealed record RawArg(string Name, IScriptObject Object); - public static readonly ArgType NONE = new None(); - - protected ArgType() { } - } - - public ArgType Arg { get; init; } = ArgType.NONE; + public RawArg? Arg { get; init; } = null; public TrellPath WorkerJs { get; init; } = TrellPath.WorkerJs; } @@ -239,32 +231,21 @@ public ScriptObject CreateJsFile(string filename, string type, byte[] contents) //var constructor = (ScriptObject)engine.Script.Uint8Array; // ScriptEngine.Current.Script.Float64Array; //var typedArray = (ITypedArray)constructor.Invoke(true, work.Arg["body"]); //work.Arg["body"] = typedArray; - var result = await Task.Run(() => work.Arg switch { - Work.ArgType.None _ => - ((IScriptObject)this.engine.Evaluate(""" + var result = await Task.Run(() => work.Arg is null + ? ((IScriptObject)this.engine.Evaluate($$""" ((hookFn, env) => hookFn({ env: JSON.parse(env), })) """ - )).InvokeAsFunction(fn, work.JsonEnv), - Work.ArgType.Raw x => - ((IScriptObject)this.engine.Evaluate($$""" + )).InvokeAsFunction(fn, work.JsonEnv) + : ((IScriptObject)this.engine.Evaluate($$""" ((hookFn, arg, env) => hookFn({ - {{x.Name}}: arg, - env: JSON.parse(env), - })) - """ - )).InvokeAsFunction(fn, x.Object, work.JsonEnv), - Work.ArgType.Json x => - ((IScriptObject)this.engine.Evaluate($$""" - ((hookFn, jsonData, env) => hookFn({ - {{x.Name}}: JSON.parse(jsonData), + {{work.Arg.Name}}: arg, env: JSON.parse(env), })) """ - )).InvokeAsFunction(fn, x.JsonString, work.JsonEnv), - _ => throw new NotSupportedException() - }); + )).InvokeAsFunction(fn, work.Arg.Object, work.JsonEnv) + ); if (result is ScriptObject so && so.GetProperty("then") is IJavaScriptObject then and { Kind: JavaScriptObjectKind.Function }) { var tcs = new TaskCompletionSource(); so.InvokeMethod("then", (object v) => { diff --git a/Trell.Test/Engine/EngineTest.cs b/Trell.Test/Engine/EngineTest.cs index 7de6ded..8413908 100644 --- a/Trell.Test/Engine/EngineTest.cs +++ b/Trell.Test/Engine/EngineTest.cs @@ -249,7 +249,7 @@ public async Task TestEngineWrapperCorrectlyCreatesAndUploadsFiles() { var work = new Work(new(), "{}", this.fixture.EngineDir, "onUpload") { WorkerJs = workerPath!, - Arg = new Work.ArgType.Raw("file", newFile), + Arg = new("file", newFile), }; var actual = await eng.RunWorkAsync(ctx, work); Assert.Equal("true", actual); diff --git a/Trell/Rpc/ToEngine.cs b/Trell/Rpc/ToEngine.cs index d8a0490..73bcf68 100644 --- a/Trell/Rpc/ToEngine.cs +++ b/Trell/Rpc/ToEngine.cs @@ -68,14 +68,14 @@ public static string ToFunctionName(this Function fn) => $"Workload must specify a function.")), }; - public static EngineWrapper.Work.ArgType ToFunctionArg(this Function fn, EngineWrapper engine) => + public static EngineWrapper.Work.RawArg ToFunctionArg(this Function fn, EngineWrapper engine) => fn?.ValueCase switch { - Function.ValueOneofCase.OnCronTrigger => new EngineWrapper.Work.ArgType.Raw("trigger", + Function.ValueOneofCase.OnCronTrigger => new EngineWrapper.Work.RawArg("trigger", engine.CreateScriptObject(new Dictionary { ["cron"] = fn.OnCronTrigger.Cron, ["timestamp"] = fn.OnCronTrigger.Timestamp.ToDateTime(), })), - Function.ValueOneofCase.OnRequest => new EngineWrapper.Work.ArgType.Raw("request", + Function.ValueOneofCase.OnRequest => new EngineWrapper.Work.RawArg("request", engine.CreateScriptObject(new Dictionary { ["url"] = fn.OnRequest.Url, ["method"] = fn.OnRequest.Method, @@ -86,11 +86,11 @@ public static EngineWrapper.Work.ArgType ToFunctionArg(this Function fn, EngineW // TODO: to a Javascript array which requires V8Engine access. //["body"] = fn.OnRequest.Body.Memory, })), - Function.ValueOneofCase.OnUpload => new EngineWrapper.Work.ArgType.Raw("file", + Function.ValueOneofCase.OnUpload => new EngineWrapper.Work.RawArg("file", engine.CreateJsFile(fn.OnUpload.Filename, fn.OnUpload.Type, fn.OnUpload.Content.ToByteArray()) ), Function.ValueOneofCase.Dynamic => - new EngineWrapper.Work.ArgType.Raw("argv", engine.CreateJsStringArray(fn.Dynamic.Arguments)), + new EngineWrapper.Work.RawArg("argv", engine.CreateJsStringArray(fn.Dynamic.Arguments)), _ => throw new TrellUserException( new TrellError( TrellErrorCode.INVALID_REQUEST, From 364549c2798df4d548cc52e0f5d24f4079c5eee9 Mon Sep 17 00:00:00 2001 From: Doug Jacob Date: Thu, 6 Feb 2025 14:16:41 -0700 Subject: [PATCH 2/2] Made the Arg property of EngineWrapper.Work required and non-nullable --- .../ClearScriptWrappers/EngineWrapper.cs | 24 +++++++------------ Trell.Test/Engine/EngineTest.cs | 8 ++++++- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/Trell.Engine/ClearScriptWrappers/EngineWrapper.cs b/Trell.Engine/ClearScriptWrappers/EngineWrapper.cs index 911391a..804c565 100644 --- a/Trell.Engine/ClearScriptWrappers/EngineWrapper.cs +++ b/Trell.Engine/ClearScriptWrappers/EngineWrapper.cs @@ -19,7 +19,7 @@ string Name ) { public sealed record RawArg(string Name, IScriptObject Object); - public RawArg? Arg { get; init; } = null; + public required RawArg Arg { get; init; } public TrellPath WorkerJs { get; init; } = TrellPath.WorkerJs; } @@ -231,20 +231,14 @@ public ScriptObject CreateJsFile(string filename, string type, byte[] contents) //var constructor = (ScriptObject)engine.Script.Uint8Array; // ScriptEngine.Current.Script.Float64Array; //var typedArray = (ITypedArray)constructor.Invoke(true, work.Arg["body"]); //work.Arg["body"] = typedArray; - var result = await Task.Run(() => work.Arg is null - ? ((IScriptObject)this.engine.Evaluate($$""" - ((hookFn, env) => hookFn({ - env: JSON.parse(env), - })) - """ - )).InvokeAsFunction(fn, work.JsonEnv) - : ((IScriptObject)this.engine.Evaluate($$""" - ((hookFn, arg, env) => hookFn({ - {{work.Arg.Name}}: arg, - env: JSON.parse(env), - })) - """ - )).InvokeAsFunction(fn, work.Arg.Object, work.JsonEnv) + var result = await Task.Run(() => + ((IScriptObject)this.engine.Evaluate($$""" + ((hookFn, arg, env) => hookFn({ + {{work.Arg.Name}}: arg, + env: JSON.parse(env), + })) + """ + )).InvokeAsFunction(fn, work.Arg.Object, work.JsonEnv) ); if (result is ScriptObject so && so.GetProperty("then") is IJavaScriptObject then and { Kind: JavaScriptObjectKind.Function }) { var tcs = new TaskCompletionSource(); diff --git a/Trell.Test/Engine/EngineTest.cs b/Trell.Test/Engine/EngineTest.cs index 8413908..6d26b04 100644 --- a/Trell.Test/Engine/EngineTest.cs +++ b/Trell.Test/Engine/EngineTest.cs @@ -200,7 +200,9 @@ public async Task TestEngineWrapperOnlyRunsPredeterminedCommands() { var eng = MakeNewEngineWrapper(); var ctx = MakeNewExecutionContext(); - var work = new Work(new(), "{}", this.fixture.EngineDir, "NotAValidFunctionName"); + var work = new Work(new(), "{}", this.fixture.EngineDir, "NotAValidFunctionName") { + Arg = new("x", eng.CreateJsStringArray(null)), + }; await Assert.ThrowsAsync(async () => await eng.RunWorkAsync(ctx, work)); string[] validFunctions = ["onCronTrigger", "onRequest", "onUpload"]; @@ -269,6 +271,7 @@ public async Task TestEngineWrapperCanTimeOut() { Assert.True(parsed); var work = new Work(new(), "{}", this.fixture.EngineDir, "onCronTrigger") { WorkerJs = workerPath!, + Arg = new("x", eng.CreateJsStringArray(null)), }; await Assert.ThrowsAsync(async () => await eng.RunWorkAsync(ctx, work)); @@ -300,6 +303,7 @@ public async Task TestEngineWrapperCanBeCanceledWithCSharpInterop() { var work = new Work(new(), "{}", this.fixture.EngineDir, "onCronTrigger") { WorkerJs = workerPath!, + Arg = new("x", eng.CreateJsStringArray(null)), }; var sw = Stopwatch.StartNew(); @@ -332,6 +336,7 @@ public async Task TestEngineWrapperCanBeCanceledWithNoCSharpInterop() { var work = new Work(new(), "{}", this.fixture.EngineDir, "onCronTrigger") { WorkerJs = workerPath!, + Arg = new("x", eng.CreateJsStringArray(null)), }; var sw = Stopwatch.StartNew(); @@ -359,6 +364,7 @@ public async Task TestWorkersCanBeTimedOutAtLoadingStep() { var work = new Work(new(), "{}", this.fixture.EngineDir, "onCronTrigger") { WorkerJs = workerPath!, + Arg = new("x", eng.CreateJsStringArray(null)), }; await Assert.ThrowsAsync(async () => await eng.RunWorkAsync(ctx, work));