From c517c272dd9e71a1a6ec00bbdab4ac4521288703 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Thu, 17 Sep 2020 19:40:07 +0200 Subject: [PATCH] Exception handling for content validation. --- backend/i18n/source/backend_en.json | 1 + .../ValidateContent/ContentValidator.cs | 9 ++- .../ValidateContent/ValidationContext.cs | 14 ++-- .../Validators/AggregateValidator.cs | 25 +++++-- .../Validators/FieldValidator.cs | 35 +++++----- .../Contents/ContentOperationContext.cs | 12 ++-- backend/src/Squidex.Shared/Texts.it.resx | 3 + backend/src/Squidex.Shared/Texts.nl.resx | 3 + backend/src/Squidex.Shared/Texts.resx | 3 + .../ValidateContent/ContentValidationTests.cs | 66 +++++++++++++++++++ .../ValidationTestExtensions.cs | 49 +++++++++++--- .../Contents/ContentDomainObjectTests.cs | 2 +- 12 files changed, 178 insertions(+), 44 deletions(-) diff --git a/backend/i18n/source/backend_en.json b/backend/i18n/source/backend_en.json index 5a5188d29..e6fddd35e 100644 --- a/backend/i18n/source/backend_en.json +++ b/backend/i18n/source/backend_en.json @@ -147,6 +147,7 @@ "contents.validation.characterCount": "Must have exactly {count} character(s).", "contents.validation.charactersBetween": "Must have between {min} and {max} character(s).", "contents.validation.duplicates": "Must not contain duplicate values.", + "contents.validation.error": "Validation failed with internal error.", "contents.validation.exactValue": "Must be exactly {value}.", "contents.validation.extension": "Must be an allowed extension.", "contents.validation.image": "Not an image.", diff --git a/backend/src/Squidex.Domain.Apps.Core.Operations/ValidateContent/ContentValidator.cs b/backend/src/Squidex.Domain.Apps.Core.Operations/ValidateContent/ContentValidator.cs index c32a10c79..955de873e 100644 --- a/backend/src/Squidex.Domain.Apps.Core.Operations/ValidateContent/ContentValidator.cs +++ b/backend/src/Squidex.Domain.Apps.Core.Operations/ValidateContent/ContentValidator.cs @@ -14,6 +14,7 @@ using Squidex.Domain.Apps.Core.Schemas; using Squidex.Domain.Apps.Core.ValidateContent.Validators; using Squidex.Infrastructure; using Squidex.Infrastructure.Json.Objects; +using Squidex.Infrastructure.Log; using Squidex.Infrastructure.Validation; #pragma warning disable SA1028, IDE0004 // Code must not contain trailing whitespace @@ -25,6 +26,7 @@ namespace Squidex.Domain.Apps.Core.ValidateContent private readonly PartitionResolver partitionResolver; private readonly ValidationContext context; private readonly IEnumerable factories; + private readonly ISemanticLog log; private readonly ConcurrentBag errors = new ConcurrentBag(); public IReadOnlyCollection Errors @@ -32,7 +34,7 @@ namespace Squidex.Domain.Apps.Core.ValidateContent get { return errors; } } - public ContentValidator(PartitionResolver partitionResolver, ValidationContext context, IEnumerable factories) + public ContentValidator(PartitionResolver partitionResolver, ValidationContext context, IEnumerable factories, ISemanticLog log) { Guard.NotNull(context, nameof(context)); Guard.NotNull(factories, nameof(factories)); @@ -40,6 +42,7 @@ namespace Squidex.Domain.Apps.Core.ValidateContent this.context = context; this.factories = factories; + this.log = log; this.partitionResolver = partitionResolver; } @@ -72,7 +75,7 @@ namespace Squidex.Domain.Apps.Core.ValidateContent { Guard.NotNull(data, nameof(data)); - var validator = new AggregateValidator(CreateContentValidators()); + var validator = new AggregateValidator(CreateContentValidators(), log); return validator.ValidateAsync(data, context, AddError); } @@ -108,7 +111,7 @@ namespace Squidex.Domain.Apps.Core.ValidateContent return new AggregateValidator( CreateFieldValidators(field) .Union(Enumerable.Repeat( - new ObjectValidator(fieldsValidators, isPartial, typeName), 1))); + new ObjectValidator(fieldsValidators, isPartial, typeName), 1)), log); } private IValidator CreateFieldValidator(IField field) diff --git a/backend/src/Squidex.Domain.Apps.Core.Operations/ValidateContent/ValidationContext.cs b/backend/src/Squidex.Domain.Apps.Core.Operations/ValidateContent/ValidationContext.cs index 1a8c660e7..720e9525b 100644 --- a/backend/src/Squidex.Domain.Apps.Core.Operations/ValidateContent/ValidationContext.cs +++ b/backend/src/Squidex.Domain.Apps.Core.Operations/ValidateContent/ValidationContext.cs @@ -48,13 +48,17 @@ namespace Squidex.Domain.Apps.Core.ValidateContent ValidationMode mode = ValidationMode.Default) { AppId = appId; + ContentId = contentId; - IsOptional = isOptional; + Mode = mode; - Path = path; Schema = schema; SchemaId = schemaId; + + IsOptional = isOptional; + + Path = path; } public ValidationContext Optimized(bool isOptimized = true) @@ -69,14 +73,14 @@ namespace Squidex.Domain.Apps.Core.ValidateContent return Clone(Path, IsOptional, mode); } - public ValidationContext Optional(bool isOptional) + public ValidationContext Optional(bool fieldIsOptional) { - if (IsOptional == isOptional) + if (IsOptional == fieldIsOptional) { return this; } - return Clone(Path, isOptional, Mode); + return Clone(Path, fieldIsOptional, Mode); } public ValidationContext Nested(string property) diff --git a/backend/src/Squidex.Domain.Apps.Core.Operations/ValidateContent/Validators/AggregateValidator.cs b/backend/src/Squidex.Domain.Apps.Core.Operations/ValidateContent/Validators/AggregateValidator.cs index 2f79b2ba6..d4d31a22e 100644 --- a/backend/src/Squidex.Domain.Apps.Core.Operations/ValidateContent/Validators/AggregateValidator.cs +++ b/backend/src/Squidex.Domain.Apps.Core.Operations/ValidateContent/Validators/AggregateValidator.cs @@ -5,29 +5,44 @@ // All rights reserved. Licensed under the MIT license. // ========================================================================== +using System; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; +using Squidex.Infrastructure.Log; +using Squidex.Infrastructure.Translations; namespace Squidex.Domain.Apps.Core.ValidateContent.Validators { public sealed class AggregateValidator : IValidator { private readonly IValidator[]? validators; + private readonly ISemanticLog log; - public AggregateValidator(IEnumerable? validators) + public AggregateValidator(IEnumerable? validators, ISemanticLog log) { this.validators = validators?.ToArray(); + + this.log = log; } - public Task ValidateAsync(object? value, ValidationContext context, AddError addError) + public async Task ValidateAsync(object? value, ValidationContext context, AddError addError) { - if (validators?.Length > 0) + try { - return Task.WhenAll(validators.Select(x => x.ValidateAsync(value, context, addError))); + if (validators?.Length > 0) + { + await Task.WhenAll(validators.Select(x => x.ValidateAsync(value, context, addError))); + } } + catch (Exception ex) + { + log.LogError(ex, w => w + .WriteProperty("action", "validateField") + .WriteProperty("status", "Failed")); - return Task.CompletedTask; + addError(context.Path, T.Get("contents.validation.error")); + } } } } diff --git a/backend/src/Squidex.Domain.Apps.Core.Operations/ValidateContent/Validators/FieldValidator.cs b/backend/src/Squidex.Domain.Apps.Core.Operations/ValidateContent/Validators/FieldValidator.cs index 892d69394..a9f7fd8c9 100644 --- a/backend/src/Squidex.Domain.Apps.Core.Operations/ValidateContent/Validators/FieldValidator.cs +++ b/backend/src/Squidex.Domain.Apps.Core.Operations/ValidateContent/Validators/FieldValidator.cs @@ -17,24 +17,24 @@ namespace Squidex.Domain.Apps.Core.ValidateContent.Validators { public sealed class FieldValidator : IValidator { - private readonly IValidator[] validators; + private readonly IValidator[]? validators; private readonly IField field; - public FieldValidator(IEnumerable validators, IField field) + public FieldValidator(IEnumerable? validators, IField field) { Guard.NotNull(field, nameof(field)); - this.validators = validators.ToArray(); + this.validators = validators?.ToArray(); this.field = field; } public async Task ValidateAsync(object? value, ValidationContext context, AddError addError) { + var typedValue = value; + try { - var typedValue = value; - if (value is IJsonValue jsonValue) { if (jsonValue.Type == JsonValueType.Null) @@ -55,22 +55,23 @@ namespace Squidex.Domain.Apps.Core.ValidateContent.Validators } } } - - if (validators?.Length > 0) - { - var tasks = new List(); - - foreach (var validator in validators) - { - tasks.Add(validator.ValidateAsync(typedValue, context, addError)); - } - - await Task.WhenAll(tasks); - } } catch { addError(context.Path, T.Get("contents.validation.invalid")); + return; + } + + if (validators?.Length > 0) + { + var tasks = new List(); + + foreach (var validator in validators) + { + tasks.Add(validator.ValidateAsync(typedValue, context, addError)); + } + + await Task.WhenAll(tasks); } } } diff --git a/backend/src/Squidex.Domain.Apps.Entities/Contents/ContentOperationContext.cs b/backend/src/Squidex.Domain.Apps.Entities/Contents/ContentOperationContext.cs index ac25cea50..7c7fb62b2 100644 --- a/backend/src/Squidex.Domain.Apps.Entities/Contents/ContentOperationContext.cs +++ b/backend/src/Squidex.Domain.Apps.Entities/Contents/ContentOperationContext.cs @@ -18,6 +18,7 @@ using Squidex.Domain.Apps.Entities.Apps; using Squidex.Domain.Apps.Entities.Contents.Commands; using Squidex.Domain.Apps.Entities.Schemas; using Squidex.Infrastructure; +using Squidex.Infrastructure.Log; using Squidex.Infrastructure.Validation; #pragma warning disable IDE0016 // Use 'throw' expression @@ -34,6 +35,7 @@ namespace Squidex.Domain.Apps.Entities.Contents }; private readonly IScriptEngine scriptEngine; + private readonly ISemanticLog log; private readonly IAppProvider appProvider; private readonly IEnumerable factories; private ISchemaEntity schema; @@ -41,11 +43,13 @@ namespace Squidex.Domain.Apps.Entities.Contents private ContentCommand command; private ValidationContext validationContext; - public ContentOperationContext(IAppProvider appProvider, IEnumerable factories, IScriptEngine scriptEngine) + public ContentOperationContext(IAppProvider appProvider, IEnumerable factories, IScriptEngine scriptEngine, ISemanticLog log) { this.appProvider = appProvider; this.factories = factories; this.scriptEngine = scriptEngine; + + this.log = log; } public ISchemaEntity Schema @@ -85,7 +89,7 @@ namespace Squidex.Domain.Apps.Entities.Contents public async Task ValidateInputAsync(NamedContentData data) { - var validator = new ContentValidator(app.PartitionResolver(), validationContext, factories); + var validator = new ContentValidator(app.PartitionResolver(), validationContext, factories, log); await validator.ValidateInputAsync(data); @@ -94,7 +98,7 @@ namespace Squidex.Domain.Apps.Entities.Contents public async Task ValidateInputPartialAsync(NamedContentData data) { - var validator = new ContentValidator(app.PartitionResolver(), validationContext, factories); + var validator = new ContentValidator(app.PartitionResolver(), validationContext, factories, log); await validator.ValidateInputPartialAsync(data); @@ -103,7 +107,7 @@ namespace Squidex.Domain.Apps.Entities.Contents public async Task ValidateContentAsync(NamedContentData data) { - var validator = new ContentValidator(app.PartitionResolver(), validationContext, factories); + var validator = new ContentValidator(app.PartitionResolver(), validationContext, factories, log); await validator.ValidateContentAsync(data); diff --git a/backend/src/Squidex.Shared/Texts.it.resx b/backend/src/Squidex.Shared/Texts.it.resx index 9af5e329f..c7d514b36 100644 --- a/backend/src/Squidex.Shared/Texts.it.resx +++ b/backend/src/Squidex.Shared/Texts.it.resx @@ -526,6 +526,9 @@ Non può avere valori duplicati. + + Validation failed with internal error. + Deve essere esattamente {value}. diff --git a/backend/src/Squidex.Shared/Texts.nl.resx b/backend/src/Squidex.Shared/Texts.nl.resx index 0465d8004..d25c00f8c 100644 --- a/backend/src/Squidex.Shared/Texts.nl.resx +++ b/backend/src/Squidex.Shared/Texts.nl.resx @@ -526,6 +526,9 @@ Mag geen dubbele waarden bevatten. + + Validation failed with internal error. + Moet exact {waarde} zijn. diff --git a/backend/src/Squidex.Shared/Texts.resx b/backend/src/Squidex.Shared/Texts.resx index 9ac34ad5a..90498f527 100644 --- a/backend/src/Squidex.Shared/Texts.resx +++ b/backend/src/Squidex.Shared/Texts.resx @@ -526,6 +526,9 @@ Must not contain duplicate values. + + Validation failed with internal error. + Must be exactly {value}. diff --git a/backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/ValidateContent/ContentValidationTests.cs b/backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/ValidateContent/ContentValidationTests.cs index f91a68215..526ebe139 100644 --- a/backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/ValidateContent/ContentValidationTests.cs +++ b/backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/ValidateContent/ContentValidationTests.cs @@ -5,13 +5,17 @@ // All rights reserved. Licensed under the MIT license. // ========================================================================== +using System; using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; +using FakeItEasy; using FluentAssertions; using Squidex.Domain.Apps.Core.Apps; using Squidex.Domain.Apps.Core.Contents; using Squidex.Domain.Apps.Core.Schemas; using Squidex.Domain.Apps.Core.TestHelpers; +using Squidex.Domain.Apps.Core.ValidateContent; using Squidex.Infrastructure; using Squidex.Infrastructure.Json.Objects; using Squidex.Infrastructure.Validation; @@ -25,6 +29,68 @@ namespace Squidex.Domain.Apps.Core.Operations.ValidateContent private readonly List errors = new List(); private Schema schema = new Schema("my-schema"); + [Fact] + public async Task Should_add_error_if_value_validator_throws_exception() + { + var validator = A.Fake(); + + A.CallTo(() => validator.ValidateAsync(A._, A._, A._)) + .Throws(new InvalidOperationException()); + + var validatorFactory = A.Fake(); + + A.CallTo(() => validatorFactory.CreateValueValidators(A._, A._, A._)) + .Returns(Enumerable.Repeat(validator, 1)); + + schema = schema.AddNumber(1, "my-field", Partitioning.Invariant, + new NumberFieldProperties()); + + var data = + new NamedContentData() + .AddField("my-field", + new ContentFieldData() + .AddValue("iv", 1000)); + + await data.ValidateAsync(languagesConfig.ToResolver(), errors, schema, factory: validatorFactory); + + errors.Should().BeEquivalentTo( + new List + { + new ValidationError("Validation failed with internal error.", "my-field") + }); + } + + [Fact] + public async Task Should_add_error_if_field_validator_throws_exception() + { + var validator = A.Fake(); + + A.CallTo(() => validator.ValidateAsync(A._, A._, A._)) + .Throws(new InvalidOperationException()); + + var validatorFactory = A.Fake(); + + A.CallTo(() => validatorFactory.CreateFieldValidators(A._, A._, A._)) + .Returns(Enumerable.Repeat(validator, 1)); + + schema = schema.AddNumber(1, "my-field", Partitioning.Invariant, + new NumberFieldProperties()); + + var data = + new NamedContentData() + .AddField("my-field", + new ContentFieldData() + .AddValue("iv", 1000)); + + await data.ValidateAsync(languagesConfig.ToResolver(), errors, schema, factory: validatorFactory); + + errors.Should().BeEquivalentTo( + new List + { + new ValidationError("Validation failed with internal error.", "my-field") + }); + } + [Fact] public async Task Should_add_error_if_validating_data_with_unknown_field() { diff --git a/backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/ValidateContent/ValidationTestExtensions.cs b/backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/ValidateContent/ValidationTestExtensions.cs index 88a79d9ca..fc648b678 100644 --- a/backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/ValidateContent/ValidationTestExtensions.cs +++ b/backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/ValidateContent/ValidationTestExtensions.cs @@ -9,23 +9,30 @@ using System; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; +using FakeItEasy; using Squidex.Domain.Apps.Core.Contents; using Squidex.Domain.Apps.Core.Schemas; using Squidex.Domain.Apps.Core.ValidateContent; using Squidex.Domain.Apps.Core.ValidateContent.Validators; using Squidex.Infrastructure; +using Squidex.Infrastructure.Log; using Squidex.Infrastructure.Validation; namespace Squidex.Domain.Apps.Core.Operations.ValidateContent { + public delegate ValidationContext ValidationUpdater(ValidationContext context); + public static class ValidationTestExtensions { private static readonly NamedId AppId = NamedId.Of(Guid.NewGuid(), "my-app"); private static readonly NamedId SchemaId = NamedId.Of(Guid.NewGuid(), "my-schema"); + private static readonly ISemanticLog Log = A.Fake(); private static readonly IValidatorsFactory Factory = new DefaultValidatorsFactory(); public static Task ValidateAsync(this IValidator validator, object? value, IList errors, - Schema? schema = null, ValidationMode mode = ValidationMode.Default, Func? updater = null) + Schema? schema = null, + ValidationMode mode = ValidationMode.Default, + ValidationUpdater? updater = null) { var context = CreateContext(schema, mode, updater); @@ -33,22 +40,28 @@ namespace Squidex.Domain.Apps.Core.Operations.ValidateContent } public static Task ValidateAsync(this IField field, object? value, IList errors, - Schema? schema = null, ValidationMode mode = ValidationMode.Default, Func? updater = null) + Schema? schema = null, + ValidationMode mode = ValidationMode.Default, + ValidationUpdater? updater = null, + IValidatorsFactory? factory = null) { var context = CreateContext(schema, mode, updater); - var validators = Factory.CreateValueValidators(context, field, null!); + var validators = Factories(factory).SelectMany(x => x.CreateValueValidators(context, field, null!)).ToArray(); - return new FieldValidator(validators.ToArray(), field) + return new FieldValidator(validators, field) .ValidateAsync(value, context, CreateFormatter(errors)); } public static async Task ValidatePartialAsync(this NamedContentData data, PartitionResolver partitionResolver, IList errors, - Schema? schema = null, ValidationMode mode = ValidationMode.Default, Func? updater = null) + Schema? schema = null, + ValidationMode mode = ValidationMode.Default, + ValidationUpdater? updater = null, + IValidatorsFactory? factory = null) { var context = CreateContext(schema, mode, updater); - var validator = new ContentValidator(partitionResolver, context, Enumerable.Repeat(Factory, 1)); + var validator = new ContentValidator(partitionResolver, context, Factories(factory), Log); await validator.ValidateInputPartialAsync(data); @@ -59,11 +72,14 @@ namespace Squidex.Domain.Apps.Core.Operations.ValidateContent } public static async Task ValidateAsync(this NamedContentData data, PartitionResolver partitionResolver, IList errors, - Schema? schema = null, ValidationMode mode = ValidationMode.Default, Func? updater = null) + Schema? schema = null, + ValidationMode mode = ValidationMode.Default, + ValidationUpdater? updater = null, + IValidatorsFactory? factory = null) { var context = CreateContext(schema, mode, updater); - var validator = new ContentValidator(partitionResolver, context, Enumerable.Repeat(Factory, 1)); + var validator = new ContentValidator(partitionResolver, context, Factories(factory), Log); await validator.ValidateInputAsync(data); @@ -88,7 +104,22 @@ namespace Squidex.Domain.Apps.Core.Operations.ValidateContent }; } - public static ValidationContext CreateContext(Schema? schema = null, ValidationMode mode = ValidationMode.Default, Func? updater = null) + private static IEnumerable Factories(IValidatorsFactory? factory) + { + var result = Enumerable.Repeat(Factory, 1); + + if (factory != null) + { + result = result.Union(Enumerable.Repeat(factory, 1)); + } + + return result; + } + + public static ValidationContext CreateContext( + Schema? schema = null, + ValidationMode mode = ValidationMode.Default, + ValidationUpdater? updater = null) { var context = new ValidationContext( AppId, diff --git a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Contents/ContentDomainObjectTests.cs b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Contents/ContentDomainObjectTests.cs index 34f22b797..9c9a8ab79 100644 --- a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Contents/ContentDomainObjectTests.cs +++ b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Contents/ContentDomainObjectTests.cs @@ -105,7 +105,7 @@ namespace Squidex.Domain.Apps.Entities.Contents patched = patch.MergeInto(data); - var context = new ContentOperationContext(appProvider, Enumerable.Repeat(new DefaultValidatorsFactory(), 1), scriptEngine); + var context = new ContentOperationContext(appProvider, Enumerable.Repeat(new DefaultValidatorsFactory(), 1), scriptEngine, A.Fake()); sut = new ContentDomainObject(Store, contentWorkflow, context, A.Dummy()); sut.Setup(Id);