From e5c40faca5d857775260239e72a1130a6076dfa7 Mon Sep 17 00:00:00 2001 From: Sebastian Stehle Date: Sun, 3 Sep 2017 20:56:44 +0200 Subject: [PATCH] Error handling improved and some tests added. Also some manual tests. --- .../Scripting/JintScriptEngine.cs | 10 +- .../Pipeline/ApiExceptionFilterAttribute.cs | 6 +- .../Scripting/ContentDataObjectTests.cs | 103 +++++++++++++++--- .../Scripting/JintScriptEngineTests.cs | 41 ++++++- 4 files changed, 137 insertions(+), 23 deletions(-) diff --git a/src/Squidex.Domain.Apps.Core/Scripting/JintScriptEngine.cs b/src/Squidex.Domain.Apps.Core/Scripting/JintScriptEngine.cs index 932db9c6c..302548d99 100644 --- a/src/Squidex.Domain.Apps.Core/Scripting/JintScriptEngine.cs +++ b/src/Squidex.Domain.Apps.Core/Scripting/JintScriptEngine.cs @@ -21,6 +21,8 @@ namespace Squidex.Domain.Apps.Core.Scripting { public sealed class JintScriptEngine : IScriptEngine { + public TimeSpan Timeout { get; set; } = TimeSpan.FromMilliseconds(200); + public void Execute(ScriptContext context, string script, string operationName) { Guard.NotNull(context, nameof(context)); @@ -86,6 +88,8 @@ namespace Squidex.Domain.Apps.Core.Scripting data.TryUpdate(out result); } })); + + engine.Execute(script); } catch (Exception) { @@ -112,9 +116,9 @@ namespace Squidex.Domain.Apps.Core.Scripting } } - private static Engine CreateScriptEngine(ScriptContext context) + private Engine CreateScriptEngine(ScriptContext context) { - var engine = new Engine(options => options.TimeoutInterval(TimeSpan.FromMilliseconds(100)).Strict()); + var engine = new Engine(options => options.TimeoutInterval(Timeout).Strict()); var contextInstance = new ObjectInstance(engine); @@ -156,7 +160,7 @@ namespace Squidex.Domain.Apps.Core.Scripting { var errors = !string.IsNullOrWhiteSpace(message) ? new[] { new ValidationError(message) } : null; - throw new ValidationException($"Script rejected to to {operationName}.", errors); + throw new ValidationException($"Script rejected to {operationName}.", errors); })); } } diff --git a/src/Squidex/Pipeline/ApiExceptionFilterAttribute.cs b/src/Squidex/Pipeline/ApiExceptionFilterAttribute.cs index c087b7628..76ed48cef 100644 --- a/src/Squidex/Pipeline/ApiExceptionFilterAttribute.cs +++ b/src/Squidex/Pipeline/ApiExceptionFilterAttribute.cs @@ -29,10 +29,10 @@ namespace Squidex.Pipeline static ApiExceptionFilterAttribute() { - AddHandler(OnDomainException); - AddHandler(OnDomainForbiddenException); - AddHandler(OnDomainObjectVersionException); AddHandler(OnDomainObjectNotFoundException); + AddHandler(OnDomainObjectVersionException); + AddHandler(OnDomainForbiddenException); + AddHandler(OnDomainException); AddHandler(OnValidationException); } diff --git a/tests/Squidex.Domain.Apps.Core.Tests/Scripting/ContentDataObjectTests.cs b/tests/Squidex.Domain.Apps.Core.Tests/Scripting/ContentDataObjectTests.cs index 0c5c33d03..6b1f265e9 100644 --- a/tests/Squidex.Domain.Apps.Core.Tests/Scripting/ContentDataObjectTests.cs +++ b/tests/Squidex.Domain.Apps.Core.Tests/Scripting/ContentDataObjectTests.cs @@ -18,7 +18,7 @@ namespace Squidex.Domain.Apps.Core.Scripting public sealed class ContentDataObjectTests { [Fact] - public void Should_update_content_when_setting_field() + public void Should_update_data_when_setting_field() { var original = new NamedContentData(); @@ -34,7 +34,31 @@ namespace Squidex.Domain.Apps.Core.Scripting } [Fact] - public void Should_update_content_when_deleting_field() + public void Should_update_data_defining_property_for_content() + { + var original = new NamedContentData(); + + var expected = + new NamedContentData() + .AddField("number", + new ContentFieldData() + .AddValue("iv", 1.0)); + + var result = ExecuteScript(original, "Object.defineProperty(data, 'number', { value: { iv: 1 } })"); + + Assert.Equal(expected, result); + } + + [Fact] + public void Should_throw_exception_when_assigning_non_object_as_field() + { + var original = new NamedContentData(); + + Assert.Throws(() => ExecuteScript(original, @"data.number = 1")); + } + + [Fact] + public void Should_update_data_when_deleting_field() { var original = new NamedContentData() @@ -50,7 +74,7 @@ namespace Squidex.Domain.Apps.Core.Scripting } [Fact] - public void Should_update_content_when_setting_field_value_with_string() + public void Should_update_data_when_setting_field_value_with_string() { var original = new NamedContentData() @@ -70,7 +94,7 @@ namespace Squidex.Domain.Apps.Core.Scripting } [Fact] - public void Should_update_content_when_setting_field_value_with_number() + public void Should_update_data_when_setting_field_value_with_number() { var original = new NamedContentData() @@ -90,7 +114,27 @@ namespace Squidex.Domain.Apps.Core.Scripting } [Fact] - public void Should_update_content_when_setting_field_value_with_array() + public void Should_update_data_when_setting_field_value_with_boolean() + { + var original = + new NamedContentData() + .AddField("boolean", + new ContentFieldData() + .AddValue("iv", false)); + + var expected = + new NamedContentData() + .AddField("boolean", + new ContentFieldData() + .AddValue("iv", true)); + + var result = ExecuteScript(original, @"data.boolean.iv = !data.boolean.iv"); + + Assert.Equal(expected, result); + } + + [Fact] + public void Should_update_data_when_setting_field_value_with_array() { var original = new NamedContentData() @@ -110,7 +154,7 @@ namespace Squidex.Domain.Apps.Core.Scripting } [Fact] - public void Should_update_content_when_setting_field_value_with_object() + public void Should_update_data_when_setting_field_value_with_object() { var original = new NamedContentData() @@ -130,27 +174,26 @@ namespace Squidex.Domain.Apps.Core.Scripting } [Fact] - public void Should_update_content_when_setting_field_value_with_boolean() + public void Should_throw_when_defining_property_for_field() { var original = new NamedContentData() - .AddField("boolean", - new ContentFieldData() - .AddValue("iv", false)); + .AddField("number", + new ContentFieldData()); var expected = new NamedContentData() - .AddField("boolean", + .AddField("number", new ContentFieldData() - .AddValue("iv", true)); + .AddValue("iv", 1.0)); - var result = ExecuteScript(original, @"data.boolean.iv = !data.boolean.iv"); + var result = ExecuteScript(original, "Object.defineProperty(data.number, 'iv', { value: 1 })"); Assert.Equal(expected, result); } [Fact] - public void Should_update_content_when_deleting_field_value() + public void Should_update_data_when_deleting_field_value() { var original = new NamedContentData() @@ -168,6 +211,38 @@ namespace Squidex.Domain.Apps.Core.Scripting Assert.Equal(expected, result); } + [Fact] + public void Should_be_able_to_iterate_over_fields() + { + var content = + new NamedContentData() + .AddField("f1", + new ContentFieldData() + .AddValue("v11", "1") + .AddValue("v12", "2")) + .AddField("f2", + new ContentFieldData() + .AddValue("v21", "3") + .AddValue("v22", "4")); + + var engine = new Engine(); + + engine.SetValue("data", new ContentDataObject(engine, content)); + + var result = engine.Execute(@" + var result = []; + for (var x in data) { + var field = data[x]; + + for (var y in field) { + result.push(field[y]); + } + } + result;").GetCompletionValue().ToObject(); + + Assert.Equal(new[] { "1", "2", "3", "4" }, result); + } + [Fact] public void Should_throw_exceptions_when_changing_objects() { diff --git a/tests/Squidex.Domain.Apps.Core.Tests/Scripting/JintScriptEngineTests.cs b/tests/Squidex.Domain.Apps.Core.Tests/Scripting/JintScriptEngineTests.cs index 168fc4b7b..84e09f2ac 100644 --- a/tests/Squidex.Domain.Apps.Core.Tests/Scripting/JintScriptEngineTests.cs +++ b/tests/Squidex.Domain.Apps.Core.Tests/Scripting/JintScriptEngineTests.cs @@ -6,6 +6,7 @@ // All rights reserved. // ========================================================================== +using System; using Squidex.Domain.Apps.Core.Contents; using Squidex.Infrastructure; using Xunit; @@ -14,7 +15,7 @@ namespace Squidex.Domain.Apps.Core.Scripting { public class JintScriptEngineTests { - private readonly JintScriptEngine scriptEngine = new JintScriptEngine(); + private readonly JintScriptEngine scriptEngine = new JintScriptEngine { Timeout = TimeSpan.FromSeconds(1) }; [Fact] public void Should_throw_validation_exception_when_calling_reject() @@ -80,7 +81,42 @@ namespace Squidex.Domain.Apps.Core.Scripting } [Fact] - public void Should_transform_content_and_return() + public void Should_transform_content_and_return_with_transform() + { + var content = + new NamedContentData() + .AddField("number0", + new ContentFieldData() + .AddValue("iv", 1.0)) + .AddField("number1", + new ContentFieldData() + .AddValue("iv", 1.0)); + var expected = + new NamedContentData() + .AddField("number1", + new ContentFieldData() + .AddValue("iv", 2.0)) + .AddField("number2", + new ContentFieldData() + .AddValue("iv", 10.0)); + + var context = new ScriptContext { Data = content }; + + var result = scriptEngine.Transform(context, @" + var data = ctx.data; + + delete data.number0; + + data.number1.iv = data.number1.iv + 1; + data.number2 = { 'iv': 10 }; + + replace(data);"); + + Assert.Equal(expected, result); + } + + [Fact] + public void Should_transform_content_and_return_with_execute_transform() { var content = new NamedContentData() @@ -112,7 +148,6 @@ namespace Squidex.Domain.Apps.Core.Scripting replace(data);", "update"); Assert.Equal(expected, result); - } } }