Skip to content

Commit dd4eb4a

Browse files
lahmaclaude
andcommitted
Map script variables on first read instead of eagerly
Every variable was mapped when the engine was set up, once per evaluation, whether or not the script ever looked at it - WritableContext did it in its constructor for the ctx path, and Engine.SetValue did it per variable for the non-context path. Some of those mappings are not cheap: a user variable walks and groups every claim, a content data variable builds a wrapper. A typical script reads a handful of the variables available to it. Both paths now defer the mapping to the first read of the value, through the two APIs Jint 4.15.3 added for exactly this: - PropertyDescriptor.CreateLazy for the ctx object. Unlike a hand-written CustomJsValue descriptor it drops the flag once the value exists, so the descriptor rejoins the write inline cache instead of paying the indirection for the rest of its life. - Engine.Advanced.AddLazyGlobal for the non-context path. The options-time AddLazyGlobal could not serve it - the variables are only known after the engine has been built - and the descriptor a host could install itself is declined by the global-identifier cache. The Advanced overload is documented as being for exactly this case, and its factory may capture engine-affine state. In both cases the property itself is installed eagerly, so nothing about the shape changes: key order, enumeration, `in`, Object.getOwnPropertyNames, delete and the write-through to ScriptVars behave exactly as before, which is what the tests pin - including a counting principal that proves the mapping has not run for a variable the script never mentions, and has run for one it reads. MapVariable reproduces Engine.SetValue's special case for a CLR type so a deferred variable cannot project differently. One edge is worth recording: Engine.SetValue writes through [[Set]] while AddLazyGlobal replaces the descriptor, so a variable named after a non-writable built-in global (undefined, NaN, Infinity) would now shadow it where it was previously ignored. ScriptVars keys are domain names, so this is not reachable in practice. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f
1 parent fe351e8 commit dd4eb4a

3 files changed

Lines changed: 177 additions & 3 deletions

File tree

backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/Internal/JintExtensions.cs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
using Jint;
99
using Jint.Native;
10+
using Jint.Runtime.Interop;
1011
using Squidex.Infrastructure;
1112

1213
namespace Squidex.Domain.Apps.Core.Scripting.Internal;
@@ -73,12 +74,29 @@ internal static ScriptExecutionContext<T> ExtendWithVariables<T>(this ScriptExec
7374
{
7475
foreach (var (key, item) in vars)
7576
{
76-
engine.SetValue(key, item);
77+
// Deferred instead of Engine.SetValue, which maps every variable now. The global itself is
78+
// installed eagerly, so existence checks and enumeration see the name without materializing
79+
// anything; only the mapping waits for the first read of the value.
80+
engine.Advanced.AddLazyGlobal(key, e => MapVariable(e, item));
7781
}
7882
}
7983

8084
engine.SetValue("async", true);
8185

8286
return context;
8387
}
88+
89+
/// <summary>
90+
/// The conversion <see cref="Engine.SetValue(string, object)"/> performs, including its special case for
91+
/// a CLR type, so deferring a variable cannot change what the script sees.
92+
/// </summary>
93+
private static JsValue MapVariable(Engine engine, object? item)
94+
{
95+
if (item is Type type)
96+
{
97+
return TypeReference.CreateTypeReference(engine, type);
98+
}
99+
100+
return JsValue.FromObject(engine, item);
101+
}
84102
}

backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/WritableContext.cs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// ==========================================================================
1+
// ==========================================================================
22
// Squidex Headless CMS
33
// ==========================================================================
44
// Copyright (c) Squidex UG (haftungsbeschraenkt)
@@ -8,6 +8,7 @@
88
using Jint;
99
using Jint.Native;
1010
using Jint.Native.Object;
11+
using Jint.Runtime.Descriptors;
1112

1213
namespace Squidex.Domain.Apps.Core.Scripting;
1314

@@ -20,9 +21,17 @@ public WritableContext(Engine engine, ScriptVars vars)
2021
{
2122
this.vars = vars;
2223

24+
// Scripts touch a fraction of the variables, but mapping one is not always cheap: a content data
25+
// variable builds a wrapper, a user variable walks and groups every claim. The descriptors are
26+
// installed eagerly - so key order, enumeration and existence checks are exactly what they were -
27+
// and only the mapping waits for the first read of a value. Once it has run the descriptor drops
28+
// back to an ordinary data property and rejoins the write inline cache, which is what a
29+
// hand-written CustomJsValue descriptor cannot do.
2330
foreach (var (key, item) in vars)
2431
{
25-
base.Set(key, FromObject(engine, item), this);
32+
SetOwnProperty(key, PropertyDescriptor.CreateLazy(
33+
(Engine: engine, Item: item),
34+
static state => FromObject(state.Engine, state.Item)));
2635
}
2736
}
2837

backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/Scripting/JintScriptEngineTests.cs

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1047,6 +1047,153 @@ public void Should_delete_context_value()
10471047
Assert.Equal(JsonValue.Create("number,json,user"), actual);
10481048
}
10491049

1050+
[Fact]
1051+
public void Should_not_map_unread_variable()
1052+
{
1053+
var principal = new CountingPrincipal();
1054+
1055+
var vars = new ScriptVars
1056+
{
1057+
["number"] = 13,
1058+
["user"] = principal,
1059+
};
1060+
1061+
const string script = @"
1062+
number + 1;
1063+
";
1064+
1065+
var actual = sut.Execute(vars, script);
1066+
1067+
Assert.Equal(JsonValue.Create(14), actual);
1068+
Assert.Equal(0, principal.Reads);
1069+
}
1070+
1071+
[Fact]
1072+
public void Should_see_unread_variable_in_enumeration()
1073+
{
1074+
var principal = new CountingPrincipal();
1075+
1076+
var vars = new ScriptVars
1077+
{
1078+
["user"] = principal,
1079+
};
1080+
1081+
const string script = @"
1082+
('user' in globalThis) + ',' + (Object.getOwnPropertyNames(globalThis).indexOf('user') >= 0);
1083+
";
1084+
1085+
var actual = sut.Execute(vars, script);
1086+
1087+
Assert.Equal(JsonValue.Create("true,true"), actual);
1088+
Assert.Equal(0, principal.Reads);
1089+
}
1090+
1091+
[Fact]
1092+
public void Should_map_variable_on_first_read()
1093+
{
1094+
var principal = new CountingPrincipal();
1095+
1096+
var vars = new ScriptVars
1097+
{
1098+
["user"] = principal,
1099+
};
1100+
1101+
const string script = @"
1102+
user.id;
1103+
";
1104+
1105+
var actual = sut.Execute(vars, script);
1106+
1107+
Assert.Equal(JsonValue.Create("user1"), actual);
1108+
Assert.True(principal.Reads > 0);
1109+
}
1110+
1111+
[Fact]
1112+
public void Should_not_map_unread_context_variable()
1113+
{
1114+
var principal = new CountingPrincipal();
1115+
1116+
var vars = new ScriptVars
1117+
{
1118+
["number"] = 13,
1119+
["user"] = principal,
1120+
};
1121+
1122+
const string script = @"
1123+
ctx.number + 1;
1124+
";
1125+
1126+
var actual = sut.Execute(vars, script, new ScriptOptions { AsContext = true });
1127+
1128+
Assert.Equal(JsonValue.Create(14), actual);
1129+
Assert.Equal(0, principal.Reads);
1130+
}
1131+
1132+
[Fact]
1133+
public void Should_see_unread_context_variable_in_enumeration()
1134+
{
1135+
var principal = new CountingPrincipal();
1136+
1137+
var vars = new ScriptVars
1138+
{
1139+
["number"] = 13,
1140+
["user"] = principal,
1141+
};
1142+
1143+
const string script = @"
1144+
Object.keys(ctx).join(',') + '|' + ('user' in ctx);
1145+
";
1146+
1147+
var actual = sut.Execute(vars, script, new ScriptOptions { AsContext = true });
1148+
1149+
Assert.Equal(JsonValue.Create("number,user|true"), actual);
1150+
Assert.Equal(0, principal.Reads);
1151+
}
1152+
1153+
[Fact]
1154+
public void Should_map_context_variable_on_first_read()
1155+
{
1156+
var principal = new CountingPrincipal();
1157+
1158+
var vars = new ScriptVars
1159+
{
1160+
["user"] = principal,
1161+
};
1162+
1163+
const string script = @"
1164+
ctx.user.id;
1165+
";
1166+
1167+
var actual = sut.Execute(vars, script, new ScriptOptions { AsContext = true });
1168+
1169+
Assert.Equal(JsonValue.Create("user1"), actual);
1170+
Assert.True(principal.Reads > 0);
1171+
}
1172+
1173+
private sealed class CountingPrincipal : ClaimsPrincipal
1174+
{
1175+
public int Reads { get; private set; }
1176+
1177+
public CountingPrincipal()
1178+
: base(new ClaimsIdentity(
1179+
[
1180+
new Claim(OpenIdClaims.Subject, "user1"),
1181+
new Claim(OpenIdClaims.Name, "user"),
1182+
], "Squidex"))
1183+
{
1184+
}
1185+
1186+
public override IEnumerable<Claim> Claims
1187+
{
1188+
get
1189+
{
1190+
Reads++;
1191+
1192+
return base.Claims;
1193+
}
1194+
}
1195+
}
1196+
10501197
private static ScriptVars CreateVars()
10511198
{
10521199
return new ScriptVars

0 commit comments

Comments
 (0)