From 3ac147a7ec163318734119326c19dcc6a09f6063 Mon Sep 17 00:00:00 2001 From: Sebastian Stehle Date: Thu, 14 Sep 2017 09:01:54 +0200 Subject: [PATCH] Bugfix: Publish script not running when publish on create --- .../Scripting/IScriptEngine.cs | 4 +- .../Scripting/JintScriptEngine.cs | 24 ++++--- .../Contents/ContentCommandMiddleware.cs | 33 +++++----- .../Scripting/JintScriptEngineTests.cs | 22 +++---- .../Contents/ContentCommandMiddlewareTests.cs | 62 ++++++++++++------- 5 files changed, 81 insertions(+), 64 deletions(-) diff --git a/src/Squidex.Domain.Apps.Core/Scripting/IScriptEngine.cs b/src/Squidex.Domain.Apps.Core/Scripting/IScriptEngine.cs index ab094ad77..bad0351f6 100644 --- a/src/Squidex.Domain.Apps.Core/Scripting/IScriptEngine.cs +++ b/src/Squidex.Domain.Apps.Core/Scripting/IScriptEngine.cs @@ -12,9 +12,9 @@ namespace Squidex.Domain.Apps.Core.Scripting { public interface IScriptEngine { - void Execute(ScriptContext context, string script, string operationName); + void Execute(ScriptContext context, string script); - NamedContentData ExecuteAndTransform(ScriptContext context, string script, string operationName); + NamedContentData ExecuteAndTransform(ScriptContext context, string script); NamedContentData Transform(ScriptContext context, string script); } diff --git a/src/Squidex.Domain.Apps.Core/Scripting/JintScriptEngine.cs b/src/Squidex.Domain.Apps.Core/Scripting/JintScriptEngine.cs index 0657c3ce2..6ab62dc73 100644 --- a/src/Squidex.Domain.Apps.Core/Scripting/JintScriptEngine.cs +++ b/src/Squidex.Domain.Apps.Core/Scripting/JintScriptEngine.cs @@ -21,7 +21,7 @@ namespace Squidex.Domain.Apps.Core.Scripting { public TimeSpan Timeout { get; set; } = TimeSpan.FromMilliseconds(200); - public void Execute(ScriptContext context, string script, string operationName) + public void Execute(ScriptContext context, string script) { Guard.NotNull(context, nameof(context)); @@ -30,13 +30,13 @@ namespace Squidex.Domain.Apps.Core.Scripting var engine = CreateScriptEngine(context); EnableDisallow(engine); - EnableReject(engine, operationName); + EnableReject(engine); - Execute(engine, script, operationName); + Execute(engine, script); } } - public NamedContentData ExecuteAndTransform(ScriptContext context, string script, string operationName) + public NamedContentData ExecuteAndTransform(ScriptContext context, string script) { Guard.NotNull(context, nameof(context)); @@ -47,7 +47,7 @@ namespace Squidex.Domain.Apps.Core.Scripting var engine = CreateScriptEngine(context); EnableDisallow(engine); - EnableReject(engine, operationName); + EnableReject(engine); engine.SetValue("operation", new Action(() => { @@ -69,7 +69,7 @@ namespace Squidex.Domain.Apps.Core.Scripting } })); - Execute(engine, script, operationName); + Execute(engine, script); } return result; @@ -108,7 +108,7 @@ namespace Squidex.Domain.Apps.Core.Scripting return result; } - private static void Execute(Engine engine, string script, string operationName) + private static void Execute(Engine engine, string script) { try { @@ -116,11 +116,11 @@ namespace Squidex.Domain.Apps.Core.Scripting } catch (ParserException ex) { - throw new ValidationException($"Failed to {operationName} with javascript syntaxs error.", new ValidationError(ex.Message)); + throw new ValidationException($"Failed to execute script with javascript syntaxs error.", new ValidationError(ex.Message)); } catch (JavaScriptException ex) { - throw new ValidationException($"Failed to {operationName} with javascript error.", new ValidationError(ex.Message)); + throw new ValidationException($"Failed to execute script with javascript error.", new ValidationError(ex.Message)); } } @@ -165,15 +165,13 @@ namespace Squidex.Domain.Apps.Core.Scripting })); } - private static void EnableReject(Engine engine, string operationName) + private static void EnableReject(Engine engine) { - Guard.NotNullOrEmpty(operationName, nameof(operationName)); - engine.SetValue("reject", new Action(message => { var errors = !string.IsNullOrWhiteSpace(message) ? new[] { new ValidationError(message) } : null; - throw new ValidationException($"Script rejected to {operationName}.", errors); + throw new ValidationException($"Script rejected the operation.", errors); })); } } diff --git a/src/Squidex.Domain.Apps.Write/Contents/ContentCommandMiddleware.cs b/src/Squidex.Domain.Apps.Write/Contents/ContentCommandMiddleware.cs index 7f4652519..23898d97d 100644 --- a/src/Squidex.Domain.Apps.Write/Contents/ContentCommandMiddleware.cs +++ b/src/Squidex.Domain.Apps.Write/Contents/ContentCommandMiddleware.cs @@ -64,9 +64,14 @@ namespace Squidex.Domain.Apps.Write.Contents await handler.CreateAsync(context, async content => { var schemaAndApp = await ResolveSchemaAndAppAsync(command); - var scriptContext = CreateScriptContext(content, command, command.Data); - command.Data = scriptEngine.ExecuteAndTransform(scriptContext, schemaAndApp.SchemaEntity.ScriptCreate, "create content"); + ExecuteScriptAndTransform(command, content, schemaAndApp.SchemaEntity.ScriptCreate, "Create"); + + if (command.Publish) + { + ExecuteScript(command, content, schemaAndApp.SchemaEntity.ScriptChange, "Published"); + } + command.Data.Enrich(schemaAndApp.SchemaEntity.SchemaDef, schemaAndApp.AppEntity.PartitionResolver); await ValidateAsync(schemaAndApp, command, () => "Failed to create content", false); @@ -82,9 +87,8 @@ namespace Squidex.Domain.Apps.Write.Contents await handler.UpdateAsync(context, async content => { var schemaAndApp = await ResolveSchemaAndAppAsync(command); - var scriptContext = CreateScriptContext(content, command, command.Data); - command.Data = scriptEngine.ExecuteAndTransform(scriptContext, schemaAndApp.SchemaEntity.ScriptUpdate, "update content"); + ExecuteScriptAndTransform(command, content, schemaAndApp.SchemaEntity.ScriptUpdate, "Update"); await ValidateAsync(schemaAndApp, command, () => "Failed to update content", false); @@ -99,9 +103,8 @@ namespace Squidex.Domain.Apps.Write.Contents await handler.UpdateAsync(context, async content => { var schemaAndApp = await ResolveSchemaAndAppAsync(command); - var scriptContext = CreateScriptContext(content, command, command.Data); - command.Data = scriptEngine.ExecuteAndTransform(scriptContext, schemaAndApp.SchemaEntity.ScriptUpdate, "patch content"); + ExecuteScriptAndTransform(command, content, schemaAndApp.SchemaEntity.ScriptUpdate, "Patch"); await ValidateAsync(schemaAndApp, command, () => "Failed to patch content", true); @@ -116,9 +119,8 @@ namespace Squidex.Domain.Apps.Write.Contents return handler.UpdateAsync(context, async content => { var schemaAndApp = await ResolveSchemaAndAppAsync(command); - var scriptContext = CreateScriptContext(content, command, command.Status.ToString()); - scriptEngine.Execute(scriptContext, schemaAndApp.SchemaEntity.ScriptChange, "change content status"); + ExecuteScript(command, content, schemaAndApp.SchemaEntity.ScriptChange, command.Status); content.ChangeStatus(command); }); @@ -129,9 +131,8 @@ namespace Squidex.Domain.Apps.Write.Contents return handler.UpdateAsync(context, async content => { var schemaAndApp = await ResolveSchemaAndAppAsync(command); - var scriptContext = CreateScriptContext(content, command, "Delete"); - scriptEngine.Execute(scriptContext, schemaAndApp.SchemaEntity.ScriptDelete, "delete content"); + ExecuteScript(command, content, schemaAndApp.SchemaEntity.ScriptDelete, "Delete"); content.Delete(command); }); @@ -179,14 +180,18 @@ namespace Squidex.Domain.Apps.Write.Contents } } - private static ScriptContext CreateScriptContext(ContentDomainObject content, ContentCommand command, string operation) + private void ExecuteScriptAndTransform(ContentDataCommand command, ContentDomainObject content, string script, object operation) { - return new ScriptContext { ContentId = content.Id, OldData = content.Data, User = command.User, Operation = operation }; + var ctx = new ScriptContext { ContentId = content.Id, OldData = content.Data, User = command.User, Operation = operation.ToString(), Data = command.Data }; + + command.Data = scriptEngine.ExecuteAndTransform(ctx, script); } - private static ScriptContext CreateScriptContext(ContentDomainObject content, ContentCommand command, NamedContentData data) + private void ExecuteScript(ContentCommand command, ContentDomainObject content, string script, object operation) { - return new ScriptContext { ContentId = content.Id, OldData = content.Data, User = command.User, Data = data }; + var ctx = new ScriptContext { ContentId = content.Id, OldData = content.Data, User = command.User, Operation = operation.ToString() }; + + scriptEngine.Execute(ctx, script); } private async Task<(ISchemaEntity SchemaEntity, IAppEntity AppEntity)> ResolveSchemaAndAppAsync(SchemaCommand command) diff --git a/tests/Squidex.Domain.Apps.Core.Tests/Scripting/JintScriptEngineTests.cs b/tests/Squidex.Domain.Apps.Core.Tests/Scripting/JintScriptEngineTests.cs index 5aea17fd4..ec13177e4 100644 --- a/tests/Squidex.Domain.Apps.Core.Tests/Scripting/JintScriptEngineTests.cs +++ b/tests/Squidex.Domain.Apps.Core.Tests/Scripting/JintScriptEngineTests.cs @@ -20,33 +20,33 @@ namespace Squidex.Domain.Apps.Core.Scripting [Fact] public void Should_throw_validation_exception_when_calling_reject() { - Assert.Throws(() => scriptEngine.Execute(new ScriptContext(), "reject()", "update")); - Assert.Throws(() => scriptEngine.Execute(new ScriptContext(), "reject('Not valid')", "update")); + Assert.Throws(() => scriptEngine.Execute(new ScriptContext(), "reject()")); + Assert.Throws(() => scriptEngine.Execute(new ScriptContext(), "reject('Not valid')")); } [Fact] public void Should_throw_security_exception_when_calling_reject() { - Assert.Throws(() => scriptEngine.Execute(new ScriptContext(), "disallow()", "Update")); - Assert.Throws(() => scriptEngine.Execute(new ScriptContext(), "disallow('Not allowed')", "update")); + Assert.Throws(() => scriptEngine.Execute(new ScriptContext(), "disallow()")); + Assert.Throws(() => scriptEngine.Execute(new ScriptContext(), "disallow('Not allowed')")); } [Fact] public void Should_catch_script_syntax_errors() { - Assert.Throws(() => scriptEngine.Execute(new ScriptContext(), "x => x", "update")); + Assert.Throws(() => scriptEngine.Execute(new ScriptContext(), "x => x")); } [Fact] public void Should_catch_script_runtime_errors() { - Assert.Throws(() => scriptEngine.Execute(new ScriptContext(), "throw 'Error';", "update")); + Assert.Throws(() => scriptEngine.Execute(new ScriptContext(), "throw 'Error';")); } [Fact] public void Should_catch_script_runtime_errors_on_execute_and_transform() { - Assert.Throws(() => scriptEngine.ExecuteAndTransform(new ScriptContext(), "throw 'Error';", "update")); + Assert.Throws(() => scriptEngine.ExecuteAndTransform(new ScriptContext(), "throw 'Error';")); } [Fact] @@ -66,7 +66,7 @@ namespace Squidex.Domain.Apps.Core.Scripting var content = new NamedContentData(); var context = new ScriptContext { Data = content }; - Assert.Throws(() => scriptEngine.ExecuteAndTransform(context, "x => x", "update")); + Assert.Throws(() => scriptEngine.ExecuteAndTransform(context, "x => x")); } [Fact] @@ -75,7 +75,7 @@ namespace Squidex.Domain.Apps.Core.Scripting var content = new NamedContentData(); var context = new ScriptContext { Data = content }; - var result = scriptEngine.ExecuteAndTransform(context, "var x = 0;", "update"); + var result = scriptEngine.ExecuteAndTransform(context, "var x = 0;"); Assert.Same(content, result); } @@ -98,7 +98,7 @@ namespace Squidex.Domain.Apps.Core.Scripting data.operation = { iv: ctx.operation }; - replace(data)", "update"); + replace(data)"); Assert.Equal(expected, result); } @@ -168,7 +168,7 @@ namespace Squidex.Domain.Apps.Core.Scripting data.number1.iv = data.number1.iv + 1; data.number2 = { 'iv': 10 }; - replace(data);", "update"); + replace(data);"); Assert.Equal(expected, result); } diff --git a/tests/Squidex.Domain.Apps.Write.Tests/Contents/ContentCommandMiddlewareTests.cs b/tests/Squidex.Domain.Apps.Write.Tests/Contents/ContentCommandMiddlewareTests.cs index 3ff6be0e0..eaf922e6d 100644 --- a/tests/Squidex.Domain.Apps.Write.Tests/Contents/ContentCommandMiddlewareTests.cs +++ b/tests/Squidex.Domain.Apps.Write.Tests/Contents/ContentCommandMiddlewareTests.cs @@ -56,16 +56,22 @@ namespace Squidex.Domain.Apps.Write.Contents A.CallTo(() => app.LanguagesConfig).Returns(languagesConfig); A.CallTo(() => app.PartitionResolver).Returns(languagesConfig.ToResolver()); + A.CallTo(() => appProvider.FindAppByIdAsync(AppId)).Returns(app); A.CallTo(() => schema.SchemaDef).Returns(schemaDef); + A.CallTo(() => schema.ScriptCreate).Returns(""); + A.CallTo(() => schema.ScriptChange).Returns(""); + A.CallTo(() => schema.ScriptUpdate).Returns(""); + A.CallTo(() => schema.ScriptDelete).Returns(""); + A.CallTo(() => schemas.FindSchemaByIdAsync(SchemaId, false)).Returns(schema); } [Fact] public async Task Create_should_throw_exception_if_data_is_not_valid() { - A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, A.Ignored, A.Ignored)) + A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, A.Ignored)) .Returns(invalidData); var context = CreateContextForCommand(new CreateContent { ContentId = contentId, Data = invalidData, User = user }); @@ -79,10 +85,8 @@ namespace Squidex.Domain.Apps.Write.Contents [Fact] public async Task Create_should_create_content() { - A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, A.Ignored, A.Ignored)) + A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, A.Ignored)) .Returns(data); - A.CallTo(() => schema.ScriptCreate) - .Returns(""); var context = CreateContextForCommand(new CreateContent { ContentId = contentId, Data = data, User = user }); @@ -93,13 +97,33 @@ namespace Squidex.Domain.Apps.Write.Contents Assert.Equal(data, context.Result>().IdOrValue); - A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, "", "create content")).MustHaveHappened(); + A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, "")).MustHaveHappened(); + A.CallTo(() => scriptEngine.Execute(A.Ignored, "")).MustNotHaveHappened(); + } + + [Fact] + public async Task Create_should_also_invoke_publish_script_when_publishing() + { + A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, A.Ignored)) + .Returns(data); + + var context = CreateContextForCommand(new CreateContent { ContentId = contentId, Data = data, User = user, Publish = true }); + + await TestCreate(content, async _ => + { + await sut.HandleAsync(context); + }); + + Assert.Equal(data, context.Result>().IdOrValue); + + A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, "")).MustHaveHappened(); + A.CallTo(() => scriptEngine.Execute(A.Ignored, "")).MustHaveHappened(); } [Fact] public async Task Update_should_throw_exception_if_data_is_not_valid() { - A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, A.Ignored, A.Ignored)).Returns(invalidData); + A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, A.Ignored)).Returns(invalidData); CreateContent(); @@ -114,10 +138,8 @@ namespace Squidex.Domain.Apps.Write.Contents [Fact] public async Task Update_should_update_domain_object() { - A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, A.Ignored, A.Ignored)) + A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, A.Ignored)) .Returns(data); - A.CallTo(() => schema.ScriptUpdate) - .Returns(""); CreateContent(); @@ -130,13 +152,13 @@ namespace Squidex.Domain.Apps.Write.Contents Assert.Equal(data, context.Result().Data); - A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, "", "update content")).MustHaveHappened(); + A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, "")).MustHaveHappened(); } [Fact] public async Task Patch_should_throw_exception_if_data_is_not_valid() { - A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, A.Ignored, A.Ignored)) + A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, A.Ignored)) .Returns(invalidData); CreateContent(); @@ -152,14 +174,12 @@ namespace Squidex.Domain.Apps.Write.Contents [Fact] public async Task Patch_should_update_domain_object() { - A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, A.Ignored, A.Ignored)) + A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, A.Ignored)) .Returns(data); - A.CallTo(() => schema.ScriptUpdate) - .Returns(""); var patch = new NamedContentData().AddField("my-field", new ContentFieldData().SetValue(3)); - A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, A.Ignored, A.Ignored)).Returns(patch); + A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, A.Ignored)).Returns(patch); CreateContent(); @@ -172,15 +192,12 @@ namespace Squidex.Domain.Apps.Write.Contents Assert.NotNull(context.Result().Data); - A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, "", "patch content")).MustHaveHappened(); + A.CallTo(() => scriptEngine.ExecuteAndTransform(A.Ignored, "")).MustHaveHappened(); } [Fact] public async Task ChangeStatus_should_publish_domain_object() { - A.CallTo(() => schema.ScriptChange) - .Returns(""); - CreateContent(); var context = CreateContextForCommand(new ChangeContentStatus { ContentId = contentId, User = user, Status = Status.Published }); @@ -190,15 +207,12 @@ namespace Squidex.Domain.Apps.Write.Contents await sut.HandleAsync(context); }); - A.CallTo(() => scriptEngine.Execute(A.Ignored, "", "change content status")).MustHaveHappened(); + A.CallTo(() => scriptEngine.Execute(A.Ignored, "")).MustHaveHappened(); } [Fact] public async Task Delete_should_update_domain_object() { - A.CallTo(() => schema.ScriptDelete) - .Returns(""); - CreateContent(); var command = CreateContextForCommand(new DeleteContent { ContentId = contentId, User = user }); @@ -208,7 +222,7 @@ namespace Squidex.Domain.Apps.Write.Contents await sut.HandleAsync(command); }); - A.CallTo(() => scriptEngine.Execute(A.Ignored, "", "delete content")).MustHaveHappened(); + A.CallTo(() => scriptEngine.Execute(A.Ignored, "")).MustHaveHappened(); } private void CreateContent()