Skip to content

Removed unnecessary arg types from EngineWrapper.Work #39

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
oconnor0 merged 2 commits into from
Feb 6, 2025
Merged

Removed unnecessary arg types from EngineWrapper.Work #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
060dec4
Removed unnecessary arg types from EngineWrapper.Work
doug-jacob Feb 6, 2025
364549c
Made the Arg property of EngineWrapper.Work required and non-nullable
doug-jacob Feb 6, 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
47 changes: 11 additions & 36 deletions Trell.Engine/ClearScriptWrappers/EngineWrapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 required RawArg Arg { get; init; }

public TrellPath WorkerJs { get; init; } = TrellPath.WorkerJs;
}
Expand Down Expand Up @@ -239,32 +231,15 @@ public ScriptObject CreateJsFile(string filename, string type, byte[] contents)
//var constructor = (ScriptObject)engine.Script.Uint8Array; // ScriptEngine.Current.Script.Float64Array;
//var typedArray = (ITypedArray<byte>)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("""
((hookFn, env) => hookFn({
env: JSON.parse(env),
}))
"""
)).InvokeAsFunction(fn, work.JsonEnv),
Work.ArgType.Raw x =>
((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),
env: JSON.parse(env),
}))
"""
)).InvokeAsFunction(fn, x.JsonString, work.JsonEnv),
_ => throw new NotSupportedException()
});
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<object?>();
so.InvokeMethod("then", (object v) => {
Expand Down
10 changes: 8 additions & 2 deletions Trell.Test/Engine/EngineTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<TrellUserException>(async () => await eng.RunWorkAsync(ctx, work));

string[] validFunctions = ["onCronTrigger", "onRequest", "onUpload"];
Expand Down Expand Up @@ -249,7 +251,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);
Expand All @@ -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<ScriptInterruptedException>(async () => await eng.RunWorkAsync(ctx, work));
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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<ScriptInterruptedException>(async () => await eng.RunWorkAsync(ctx, work));
Expand Down
10 changes: 5 additions & 5 deletions Trell/Rpc/ToEngine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, object> {
["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<string, object> {
["url"] = fn.OnRequest.Url,
["method"] = fn.OnRequest.Method,
Expand All @@ -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,
Expand Down