From 60ab010172498c531fd356fa76f122ece9b57a44 Mon Sep 17 00:00:00 2001 From: Sebastian Stehle Date: Tue, 30 Apr 2019 14:46:24 +0200 Subject: [PATCH] Fix for error handling. --- .../ValidateContent/ContentValidator.cs | 2 +- .../Orleans/LoggingFilter.cs | 44 +++++++++++++++++ .../ValidationException.cs | 21 ++++---- .../Frontend/Middlewares/WebpackMiddleware.cs | 42 ++++++++++++---- src/Squidex/Config/Orleans/OrleansServices.cs | 1 + src/Squidex/package-lock.json | 21 +++++--- .../ValidateContent/ContentValidationTests.cs | 38 +++++++------- .../Orleans/LoggingFilterTests.cs | 49 +++++++++++++++++++ .../ValidationExceptionTests.cs | 12 +++++ 9 files changed, 185 insertions(+), 45 deletions(-) create mode 100644 src/Squidex.Infrastructure/Orleans/LoggingFilter.cs create mode 100644 tests/Squidex.Infrastructure.Tests/Orleans/LoggingFilterTests.cs diff --git a/src/Squidex.Domain.Apps.Core.Operations/ValidateContent/ContentValidator.cs b/src/Squidex.Domain.Apps.Core.Operations/ValidateContent/ContentValidator.cs index 11b072b8c..e67652f9c 100644 --- a/src/Squidex.Domain.Apps.Core.Operations/ValidateContent/ContentValidator.cs +++ b/src/Squidex.Domain.Apps.Core.Operations/ValidateContent/ContentValidator.cs @@ -46,7 +46,7 @@ namespace Squidex.Domain.Apps.Core.ValidateContent { var pathString = path.ToPathString(); - errors.Add(new ValidationError($"{pathString}: {message}", pathString)); + errors.Add(new ValidationError(message, pathString)); } public Task ValidatePartialAsync(NamedContentData data) diff --git a/src/Squidex.Infrastructure/Orleans/LoggingFilter.cs b/src/Squidex.Infrastructure/Orleans/LoggingFilter.cs new file mode 100644 index 000000000..e52c1fef2 --- /dev/null +++ b/src/Squidex.Infrastructure/Orleans/LoggingFilter.cs @@ -0,0 +1,44 @@ +// ========================================================================== +// Squidex Headless CMS +// ========================================================================== +// Copyright (c) Squidex UG (haftungsbeschraenkt) +// All rights reserved. Licensed under the MIT license. +// ========================================================================== + +using System; +using System.Threading.Tasks; +using Orleans; +using Squidex.Infrastructure.Log; + +namespace Squidex.Infrastructure.Orleans +{ + public sealed class LoggingFilter : IIncomingGrainCallFilter + { + private readonly ISemanticLog log; + + public LoggingFilter(ISemanticLog log) + { + Guard.NotNull(log, nameof(log)); + + this.log = log; + } + + public async Task Invoke(IIncomingGrainCallContext context) + { + try + { + await context.Invoke(); + } + catch (Exception ex) + { + log.LogError(ex, w => w + .WriteProperty("action", "GrainInvoked") + .WriteProperty("status", "Failed") + .WriteProperty("grain", context.Grain.ToString()) + .WriteProperty("grainMethod", context.ImplementationMethod.ToString())); + + throw; + } + } + } +} diff --git a/src/Squidex.Infrastructure/ValidationException.cs b/src/Squidex.Infrastructure/ValidationException.cs index 16f3d836c..3f1b49c1e 100644 --- a/src/Squidex.Infrastructure/ValidationException.cs +++ b/src/Squidex.Infrastructure/ValidationException.cs @@ -78,18 +78,21 @@ namespace Squidex.Infrastructure for (var i = 0; i < errors.Count; i++) { - var error = errors[i].Message; + var error = errors[i]?.Message; - sb.Append(error); - - if (!error.EndsWith(".", StringComparison.OrdinalIgnoreCase)) + if (!string.IsNullOrWhiteSpace(error)) { - sb.Append("."); - } + sb.Append(error); - if (i < errors.Count - 1) - { - sb.Append(" "); + if (!error.EndsWith(".", StringComparison.OrdinalIgnoreCase)) + { + sb.Append("."); + } + + if (i < errors.Count - 1) + { + sb.Append(" "); + } } } } diff --git a/src/Squidex/Areas/Frontend/Middlewares/WebpackMiddleware.cs b/src/Squidex/Areas/Frontend/Middlewares/WebpackMiddleware.cs index e6731f739..7c0546ae5 100644 --- a/src/Squidex/Areas/Frontend/Middlewares/WebpackMiddleware.cs +++ b/src/Squidex/Areas/Frontend/Middlewares/WebpackMiddleware.cs @@ -5,7 +5,9 @@ // All rights reserved. Licensed under the MIT license. // ========================================================================== +using System.IO; using System.Net.Http; +using System.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; @@ -23,7 +25,7 @@ namespace Squidex.Areas.Frontend.Middlewares public async Task Invoke(HttpContext context) { - if (context.IsHtmlPath()) + if (context.IsIndex()) { if (context.Response.StatusCode != 304) { @@ -37,27 +39,49 @@ namespace Squidex.Areas.Frontend.Middlewares { var html = await result.Content.ReadAsStringAsync(); - var basePath = context.Request.PathBase; - - if (basePath.HasValue) - { - html = AdjustBase(html, basePath.Value); - } + html = AdjustBase(html, context.Request.PathBase); await context.Response.WriteHtmlAsync(html); } } } } + else if (context.IsHtmlPath()) + { + var responseBuffer = new MemoryStream(); + var responseBody = context.Response.Body; + + context.Response.Body = responseBuffer; + + await next(context); + + context.Response.Body = responseBody; + + var html = Encoding.UTF8.GetString(responseBuffer.ToArray()); + + html = AdjustBase(html, context.Request.PathBase); + + context.Response.ContentLength = Encoding.UTF8.GetByteCount(html); + context.Response.Body = responseBody; + + await context.Response.WriteAsync(html); + } else { await next(context); } } - private static string AdjustBase(string response, string baseUrl) + private static string AdjustBase(string html, PathString baseUrl) { - return response.Replace("", $""); + if (baseUrl.HasValue) + { + return html.Replace("", $""); + } + else + { + return html; + } } } } diff --git a/src/Squidex/Config/Orleans/OrleansServices.cs b/src/Squidex/Config/Orleans/OrleansServices.cs index 248f98580..ace29d8ee 100644 --- a/src/Squidex/Config/Orleans/OrleansServices.cs +++ b/src/Squidex/Config/Orleans/OrleansServices.cs @@ -36,6 +36,7 @@ namespace Squidex.Config.Orleans builder.ConfigureServices(siloServices => { siloServices.AddSingleton(); + siloServices.AddSingleton(); }); builder.ConfigureApplicationParts(parts => diff --git a/src/Squidex/package-lock.json b/src/Squidex/package-lock.json index 024dec9f8..addc1d0f2 100644 --- a/src/Squidex/package-lock.json +++ b/src/Squidex/package-lock.json @@ -1288,6 +1288,7 @@ "resolved": "https://registry.npmjs.org/align-text/-/align-text-0.1.4.tgz", "integrity": "sha1-DNkKVhCT810KmSVsIrcGlDP60Rc=", "dev": true, + "optional": true, "requires": { "kind-of": "^3.0.2", "longest": "^1.0.1", @@ -3460,7 +3461,7 @@ }, "json5": { "version": "1.0.1", - "resolved": "http://registry.npmjs.org/json5/-/json5-1.0.1.tgz", + "resolved": "https://registry.npmjs.org/json5/-/json5-1.0.1.tgz", "integrity": "sha512-aKS4WQjPenRxiQsC93MNfjx+nbF4PAdYzmd/1JIj8HYzqfbu86beTuNgXDzPknWk0n0uARlyewZo4s++ES36Ow==", "dev": true, "requires": { @@ -5496,7 +5497,8 @@ "ansi-regex": { "version": "2.1.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "aproba": { "version": "1.2.0", @@ -5911,7 +5913,8 @@ "safe-buffer": { "version": "5.1.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "safer-buffer": { "version": "2.1.2", @@ -5967,6 +5970,7 @@ "version": "3.0.1", "bundled": true, "dev": true, + "optional": true, "requires": { "ansi-regex": "^2.0.0" } @@ -6010,12 +6014,14 @@ "wrappy": { "version": "1.0.2", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "yallist": { "version": "3.0.2", "bundled": true, - "dev": true + "dev": true, + "optional": true } } }, @@ -8731,7 +8737,8 @@ "version": "1.0.1", "resolved": "https://registry.npmjs.org/longest/-/longest-1.0.1.tgz", "integrity": "sha1-MKCy2jj3N3DoKUoNIuZiXtd9AJc=", - "dev": true + "dev": true, + "optional": true }, "loose-envify": { "version": "1.4.0", @@ -9632,7 +9639,7 @@ }, "onetime": { "version": "1.1.0", - "resolved": "http://registry.npmjs.org/onetime/-/onetime-1.1.0.tgz", + "resolved": "https://registry.npmjs.org/onetime/-/onetime-1.1.0.tgz", "integrity": "sha1-ofeDj4MUxRbwXs78vEzP4EtO14k=", "dev": true }, diff --git a/tests/Squidex.Domain.Apps.Core.Tests/Operations/ValidateContent/ContentValidationTests.cs b/tests/Squidex.Domain.Apps.Core.Tests/Operations/ValidateContent/ContentValidationTests.cs index d6b26b049..1c1601934 100644 --- a/tests/Squidex.Domain.Apps.Core.Tests/Operations/ValidateContent/ContentValidationTests.cs +++ b/tests/Squidex.Domain.Apps.Core.Tests/Operations/ValidateContent/ContentValidationTests.cs @@ -38,7 +38,7 @@ namespace Squidex.Domain.Apps.Core.Operations.ValidateContent errors.Should().BeEquivalentTo( new List { - new ValidationError("unknown: Not a known field.", "unknown") + new ValidationError("Not a known field.", "unknown") }); } @@ -59,7 +59,7 @@ namespace Squidex.Domain.Apps.Core.Operations.ValidateContent errors.Should().BeEquivalentTo( new List { - new ValidationError("my-field: Must be less or equal to '100'.", "my-field") + new ValidationError("Must be less or equal to '100'.", "my-field") }); } @@ -80,8 +80,8 @@ namespace Squidex.Domain.Apps.Core.Operations.ValidateContent errors.Should().BeEquivalentTo( new List { - new ValidationError("my-field(es): Not a known invariant value.", "my-field(es)"), - new ValidationError("my-field(it): Not a known invariant value.", "my-field(it)") + new ValidationError("Not a known invariant value.", "my-field(es)"), + new ValidationError("Not a known invariant value.", "my-field(it)") }); } @@ -99,8 +99,8 @@ namespace Squidex.Domain.Apps.Core.Operations.ValidateContent errors.Should().BeEquivalentTo( new List { - new ValidationError("my-field(de): Field is required.", "my-field(de)"), - new ValidationError("my-field(en): Field is required.", "my-field(en)") + new ValidationError("Field is required.", "my-field(de)"), + new ValidationError("Field is required.", "my-field(en)") }); } @@ -118,7 +118,7 @@ namespace Squidex.Domain.Apps.Core.Operations.ValidateContent errors.Should().BeEquivalentTo( new List { - new ValidationError("my-field: Field is required.", "my-field") + new ValidationError("Field is required.", "my-field") }); } @@ -139,7 +139,7 @@ namespace Squidex.Domain.Apps.Core.Operations.ValidateContent errors.Should().BeEquivalentTo( new List { - new ValidationError("my-field(xx): Not a known language.", "my-field(xx)") + new ValidationError("Not a known language.", "my-field(xx)") }); } @@ -182,8 +182,8 @@ namespace Squidex.Domain.Apps.Core.Operations.ValidateContent errors.Should().BeEquivalentTo( new List { - new ValidationError("my-field(es): Not a known language.", "my-field(es)"), - new ValidationError("my-field(it): Not a known language.", "my-field(it)") + new ValidationError("Not a known language.", "my-field(es)"), + new ValidationError("Not a known language.", "my-field(it)") }); } @@ -200,7 +200,7 @@ namespace Squidex.Domain.Apps.Core.Operations.ValidateContent errors.Should().BeEquivalentTo( new List { - new ValidationError("unknown: Not a known field.", "unknown") + new ValidationError("Not a known field.", "unknown") }); } @@ -221,7 +221,7 @@ namespace Squidex.Domain.Apps.Core.Operations.ValidateContent errors.Should().BeEquivalentTo( new List { - new ValidationError("my-field: Must be less or equal to '100'.", "my-field") + new ValidationError("Must be less or equal to '100'.", "my-field") }); } @@ -242,8 +242,8 @@ namespace Squidex.Domain.Apps.Core.Operations.ValidateContent errors.Should().BeEquivalentTo( new List { - new ValidationError("my-field(es): Not a known invariant value.", "my-field(es)"), - new ValidationError("my-field(it): Not a known invariant value.", "my-field(it)") + new ValidationError("Not a known invariant value.", "my-field(es)"), + new ValidationError("Not a known invariant value.", "my-field(it)") }); } @@ -292,7 +292,7 @@ namespace Squidex.Domain.Apps.Core.Operations.ValidateContent errors.Should().BeEquivalentTo( new List { - new ValidationError("my-field(xx): Not a known language.", "my-field(xx)") + new ValidationError("Not a known language.", "my-field(xx)") }); } @@ -313,8 +313,8 @@ namespace Squidex.Domain.Apps.Core.Operations.ValidateContent errors.Should().BeEquivalentTo( new List { - new ValidationError("my-field(es): Not a known language.", "my-field(es)"), - new ValidationError("my-field(it): Not a known language.", "my-field(it)") + new ValidationError("Not a known language.", "my-field(es)"), + new ValidationError("Not a known language.", "my-field(it)") }); } @@ -340,8 +340,8 @@ namespace Squidex.Domain.Apps.Core.Operations.ValidateContent errors.Should().BeEquivalentTo( new List { - new ValidationError("my-field[1].my-nested: Field is required.", "my-field[1].my-nested"), - new ValidationError("my-field[3].my-nested: Field is required.", "my-field[3].my-nested") + new ValidationError("Field is required.", "my-field[1].my-nested"), + new ValidationError("Field is required.", "my-field[3].my-nested") }); } } diff --git a/tests/Squidex.Infrastructure.Tests/Orleans/LoggingFilterTests.cs b/tests/Squidex.Infrastructure.Tests/Orleans/LoggingFilterTests.cs new file mode 100644 index 000000000..d3e646f1e --- /dev/null +++ b/tests/Squidex.Infrastructure.Tests/Orleans/LoggingFilterTests.cs @@ -0,0 +1,49 @@ +// ========================================================================== +// Squidex Headless CMS +// ========================================================================== +// Copyright (c) Squidex UG (haftungsbeschraenkt) +// All rights reserved. Licensed under the MIT license. +// ========================================================================== + +using System; +using System.Threading.Tasks; +using FakeItEasy; +using Orleans; +using Squidex.Infrastructure.Log; +using Xunit; + +namespace Squidex.Infrastructure.Orleans +{ + public class LoggingFilterTests + { + private readonly ISemanticLog log = A.Fake(); + private readonly IIncomingGrainCallContext context = A.Fake(); + private readonly LoggingFilter sut; + + public LoggingFilterTests() + { + sut = new LoggingFilter(log); + } + + [Fact] + public async Task Should_not_log_if_no_exception_happened() + { + await sut.Invoke(context); + + A.CallTo(() => log.Log(A.Ignored, A.Ignored, A>.Ignored)) + .MustNotHaveHappened(); + } + + [Fact] + public async Task Should_log_exception_and_forward_it() + { + A.CallTo(() => context.Invoke()) + .Throws(new InvalidOperationException()); + + await Assert.ThrowsAsync(() => sut.Invoke(context)); + + A.CallTo(() => log.Log(A.Ignored, A.Ignored, A>.Ignored)) + .MustHaveHappened(); + } + } +} diff --git a/tests/Squidex.Infrastructure.Tests/ValidationExceptionTests.cs b/tests/Squidex.Infrastructure.Tests/ValidationExceptionTests.cs index 9acb88b7a..8d2df1fbc 100644 --- a/tests/Squidex.Infrastructure.Tests/ValidationExceptionTests.cs +++ b/tests/Squidex.Infrastructure.Tests/ValidationExceptionTests.cs @@ -53,6 +53,18 @@ namespace Squidex.Infrastructure Assert.Equal("Summary: Error1. Error2.", ex.Message); } + [Fact] + public void Should_serialize_and_deserialize1() + { + var source = new ValidationException("Summary", new ValidationError("Error1"), null); + var result = source.SerializeAndDeserializeBinary(); + + result.Errors.Should().BeEquivalentTo(source.Errors); + + Assert.Equal(source.Message, result.Message); + Assert.Equal(source.Summary, result.Summary); + } + [Fact] public void Should_serialize_and_deserialize() {