From 530e66850e4744f5f0350ff3616fe1828772ce8d Mon Sep 17 00:00:00 2001 From: Sebastian Stehle Date: Tue, 30 Apr 2019 11:41:02 +0200 Subject: [PATCH 1/3] Hopefully improved the logging. --- .../Frontend/Middlewares/WebpackMiddleware.cs | 86 ++++--------------- src/Squidex/Config/Logging.cs | 2 +- src/Squidex/Config/Orleans/OrleansServices.cs | 5 ++ src/Squidex/Program.cs | 2 +- 4 files changed, 26 insertions(+), 69 deletions(-) diff --git a/src/Squidex/Areas/Frontend/Middlewares/WebpackMiddleware.cs b/src/Squidex/Areas/Frontend/Middlewares/WebpackMiddleware.cs index 2dc98b9bb..e6731f739 100644 --- a/src/Squidex/Areas/Frontend/Middlewares/WebpackMiddleware.cs +++ b/src/Squidex/Areas/Frontend/Middlewares/WebpackMiddleware.cs @@ -5,9 +5,7 @@ // All rights reserved. Licensed under the MIT license. // ========================================================================== -using System; -using System.IO; -using System.Text; +using System.Net.Http; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; @@ -15,10 +13,7 @@ namespace Squidex.Areas.Frontend.Middlewares { public sealed class WebpackMiddleware { - private const string Host = "localhost"; - private const string Port = "3000"; - private static readonly string[] Scripts = { "shims", "app" }; - private static readonly string[] Styles = Array.Empty(); + private const string WebpackUrl = "http://localhost:3000/index.html"; private readonly RequestDelegate next; public WebpackMiddleware(RequestDelegate next) @@ -30,34 +25,29 @@ namespace Squidex.Areas.Frontend.Middlewares { if (context.IsHtmlPath()) { - var responseBuffer = new MemoryStream(); - var responseBody = context.Response.Body; - - context.Response.Body = responseBuffer; - - await next(context); + if (context.Response.StatusCode != 304) + { + using (var client = new HttpClient()) + { + var result = await client.GetAsync(WebpackUrl); - context.Response.Body = responseBody; + context.Response.StatusCode = (int)result.StatusCode; - var response = Encoding.UTF8.GetString(responseBuffer.ToArray()); + if (result.IsSuccessStatusCode) + { + var html = await result.Content.ReadAsStringAsync(); - if (context.IsIndex()) - { - response = InjectStyles(response); - response = InjectScripts(response); - } + var basePath = context.Request.PathBase; - var basePath = context.Request.PathBase; + if (basePath.HasValue) + { + html = AdjustBase(html, basePath.Value); + } - if (basePath.HasValue) - { - response = AdjustBase(response, basePath.Value); + await context.Response.WriteHtmlAsync(html); + } + } } - - context.Response.ContentLength = Encoding.UTF8.GetByteCount(response); - context.Response.Body = responseBody; - - await context.Response.WriteAsync(response); } else { @@ -65,44 +55,6 @@ namespace Squidex.Areas.Frontend.Middlewares } } - private static string InjectStyles(string response) - { - if (!response.Contains("")) - { - return response; - } - - var sb = new StringBuilder(); - - foreach (var file in Styles) - { - sb.AppendLine($""); - } - - response = response.Replace("", $"{sb}"); - - return response; - } - - private static string InjectScripts(string response) - { - if (!response.Contains("")) - { - return response; - } - - var sb = new StringBuilder(); - - foreach (var file in Scripts) - { - sb.AppendLine($""); - } - - response = response.Replace("", $"{sb}"); - - return response; - } - private static string AdjustBase(string response, string baseUrl) { return response.Replace("", $""); diff --git a/src/Squidex/Config/Logging.cs b/src/Squidex/Config/Logging.cs index 76a2b22ea..42e3a0ecc 100644 --- a/src/Squidex/Config/Logging.cs +++ b/src/Squidex/Config/Logging.cs @@ -14,7 +14,7 @@ namespace Squidex.Config { public static class Logging { - public static void AddFilter(this ILoggingBuilder builder) + public static void AddFilters(this ILoggingBuilder builder) { builder.AddFilter((category, level) => { diff --git a/src/Squidex/Config/Orleans/OrleansServices.cs b/src/Squidex/Config/Orleans/OrleansServices.cs index 458dc225a..248f98580 100644 --- a/src/Squidex/Config/Orleans/OrleansServices.cs +++ b/src/Squidex/Config/Orleans/OrleansServices.cs @@ -28,6 +28,11 @@ namespace Squidex.Config.Orleans { services.AddOrleans(config, environment, builder => { + builder.ConfigureLogging(logging => + { + logging.AddFilters(); + }); + builder.ConfigureServices(siloServices => { siloServices.AddSingleton(); diff --git a/src/Squidex/Program.cs b/src/Squidex/Program.cs index e427eff3d..dd6f6abd7 100644 --- a/src/Squidex/Program.cs +++ b/src/Squidex/Program.cs @@ -31,7 +31,7 @@ namespace Squidex { builder.AddConfiguration(hostingContext.Configuration.GetSection("logging")); builder.AddSemanticLog(); - builder.AddFilter(); + builder.AddFilters(); }) .ConfigureAppConfiguration((hostContext, builder) => { From 60ab010172498c531fd356fa76f122ece9b57a44 Mon Sep 17 00:00:00 2001 From: Sebastian Stehle Date: Tue, 30 Apr 2019 14:46:24 +0200 Subject: [PATCH 2/3] 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() { From 8da37979fbc014b3ffc1b3dccb757db98aa810ce Mon Sep 17 00:00:00 2001 From: Sebastian Stehle Date: Tue, 30 Apr 2019 15:02:41 +0200 Subject: [PATCH 3/3] Do not log domain exceptions. --- src/Squidex.Infrastructure/Orleans/LoggingFilter.cs | 4 ++++ .../Orleans/LoggingFilterTests.cs | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/src/Squidex.Infrastructure/Orleans/LoggingFilter.cs b/src/Squidex.Infrastructure/Orleans/LoggingFilter.cs index e52c1fef2..b7edc178c 100644 --- a/src/Squidex.Infrastructure/Orleans/LoggingFilter.cs +++ b/src/Squidex.Infrastructure/Orleans/LoggingFilter.cs @@ -29,6 +29,10 @@ namespace Squidex.Infrastructure.Orleans { await context.Invoke(); } + catch (DomainException) + { + throw; + } catch (Exception ex) { log.LogError(ex, w => w diff --git a/tests/Squidex.Infrastructure.Tests/Orleans/LoggingFilterTests.cs b/tests/Squidex.Infrastructure.Tests/Orleans/LoggingFilterTests.cs index d3e646f1e..28ee105e9 100644 --- a/tests/Squidex.Infrastructure.Tests/Orleans/LoggingFilterTests.cs +++ b/tests/Squidex.Infrastructure.Tests/Orleans/LoggingFilterTests.cs @@ -34,6 +34,18 @@ namespace Squidex.Infrastructure.Orleans .MustNotHaveHappened(); } + [Fact] + public async Task Should_not_log_domain_exceptions() + { + A.CallTo(() => context.Invoke()) + .Throws(new ValidationException("Failed")); + + await Assert.ThrowsAsync(() => sut.Invoke(context)); + + A.CallTo(() => log.Log(A.Ignored, A.Ignored, A>.Ignored)) + .MustNotHaveHappened(); + } + [Fact] public async Task Should_log_exception_and_forward_it() {