Browse Source

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
pull/1326/head
Marko Lahma 1 month ago
parent
commit
8a8cd6625c
  1. 20
      backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/Internal/JintExtensions.cs
  2. 13
      backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/WritableContext.cs
  3. 147
      backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/Scripting/JintScriptEngineTests.cs

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

@ -7,6 +7,7 @@
using Jint; using Jint;
using Jint.Native; using Jint.Native;
using Jint.Runtime.Interop;
using Squidex.Infrastructure; using Squidex.Infrastructure;
namespace Squidex.Domain.Apps.Core.Scripting.Internal; namespace Squidex.Domain.Apps.Core.Scripting.Internal;
@ -73,7 +74,10 @@ public static class JintExtensions
{ {
foreach (var (key, item) in vars) foreach (var (key, item) in vars)
{ {
engine.SetValue(key, item); // Deferred instead of Engine.SetValue, which maps every variable now. The global itself is
// installed eagerly, so existence checks and enumeration see the name without materializing
// anything; only the mapping waits for the first read of the value.
engine.Advanced.AddLazyGlobal(key, e => MapVariable(e, item));
} }
} }
@ -81,4 +85,18 @@ public static class JintExtensions
return context; return context;
} }
/// <summary>
/// The conversion <see cref="Engine.SetValue(string, object)"/> performs, including its special case for
/// a CLR type, so deferring a variable cannot change what the script sees.
/// </summary>
private static JsValue MapVariable(Engine engine, object? item)
{
if (item is Type type)
{
return TypeReference.CreateTypeReference(engine, type);
}
return JsValue.FromObject(engine, item);
}
} }

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

@ -1,4 +1,4 @@
// ========================================================================== // ==========================================================================
// Squidex Headless CMS // Squidex Headless CMS
// ========================================================================== // ==========================================================================
// Copyright (c) Squidex UG (haftungsbeschraenkt) // Copyright (c) Squidex UG (haftungsbeschraenkt)
@ -8,6 +8,7 @@
using Jint; using Jint;
using Jint.Native; using Jint.Native;
using Jint.Native.Object; using Jint.Native.Object;
using Jint.Runtime.Descriptors;
namespace Squidex.Domain.Apps.Core.Scripting; namespace Squidex.Domain.Apps.Core.Scripting;
@ -20,9 +21,17 @@ internal sealed class WritableContext : ObjectInstance
{ {
this.vars = vars; this.vars = vars;
// Scripts touch a fraction of the variables, but mapping one is not always cheap: a content data
// variable builds a wrapper, a user variable walks and groups every claim. The descriptors are
// installed eagerly - so key order, enumeration and existence checks are exactly what they were -
// and only the mapping waits for the first read of a value. Once it has run the descriptor drops
// back to an ordinary data property and rejoins the write inline cache, which is what a
// hand-written CustomJsValue descriptor cannot do.
foreach (var (key, item) in vars) foreach (var (key, item) in vars)
{ {
base.Set(key, FromObject(engine, item), this); SetOwnProperty(key, PropertyDescriptor.CreateLazy(
(Engine: engine, Item: item),
static state => FromObject(state.Engine, state.Item)));
} }
} }

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

@ -1047,6 +1047,153 @@ public class JintScriptEngineTests : IClassFixture<TranslationsFixture>
Assert.Equal(JsonValue.Create("number,json,user"), actual); Assert.Equal(JsonValue.Create("number,json,user"), actual);
} }
[Fact]
public void Should_not_map_unread_variable()
{
var principal = new CountingPrincipal();
var vars = new ScriptVars
{
["number"] = 13,
["user"] = principal,
};
const string script = @"
number + 1;
";
var actual = sut.Execute(vars, script);
Assert.Equal(JsonValue.Create(14), actual);
Assert.Equal(0, principal.Reads);
}
[Fact]
public void Should_see_unread_variable_in_enumeration()
{
var principal = new CountingPrincipal();
var vars = new ScriptVars
{
["user"] = principal,
};
const string script = @"
('user' in globalThis) + ',' + (Object.getOwnPropertyNames(globalThis).indexOf('user') >= 0);
";
var actual = sut.Execute(vars, script);
Assert.Equal(JsonValue.Create("true,true"), actual);
Assert.Equal(0, principal.Reads);
}
[Fact]
public void Should_map_variable_on_first_read()
{
var principal = new CountingPrincipal();
var vars = new ScriptVars
{
["user"] = principal,
};
const string script = @"
user.id;
";
var actual = sut.Execute(vars, script);
Assert.Equal(JsonValue.Create("user1"), actual);
Assert.True(principal.Reads > 0);
}
[Fact]
public void Should_not_map_unread_context_variable()
{
var principal = new CountingPrincipal();
var vars = new ScriptVars
{
["number"] = 13,
["user"] = principal,
};
const string script = @"
ctx.number + 1;
";
var actual = sut.Execute(vars, script, new ScriptOptions { AsContext = true });
Assert.Equal(JsonValue.Create(14), actual);
Assert.Equal(0, principal.Reads);
}
[Fact]
public void Should_see_unread_context_variable_in_enumeration()
{
var principal = new CountingPrincipal();
var vars = new ScriptVars
{
["number"] = 13,
["user"] = principal,
};
const string script = @"
Object.keys(ctx).join(',') + '|' + ('user' in ctx);
";
var actual = sut.Execute(vars, script, new ScriptOptions { AsContext = true });
Assert.Equal(JsonValue.Create("number,user|true"), actual);
Assert.Equal(0, principal.Reads);
}
[Fact]
public void Should_map_context_variable_on_first_read()
{
var principal = new CountingPrincipal();
var vars = new ScriptVars
{
["user"] = principal,
};
const string script = @"
ctx.user.id;
";
var actual = sut.Execute(vars, script, new ScriptOptions { AsContext = true });
Assert.Equal(JsonValue.Create("user1"), actual);
Assert.True(principal.Reads > 0);
}
private sealed class CountingPrincipal : ClaimsPrincipal
{
public int Reads { get; private set; }
public CountingPrincipal()
: base(new ClaimsIdentity(
[
new Claim(OpenIdClaims.Subject, "user1"),
new Claim(OpenIdClaims.Name, "user"),
], "Squidex"))
{
}
public override IEnumerable<Claim> Claims
{
get
{
Reads++;
return base.Claims;
}
}
}
private static ScriptVars CreateVars() private static ScriptVars CreateVars()
{ {
return new ScriptVars return new ScriptVars

Loading…
Cancel
Save