From b6282793a6ce22b5f6ecf12f586298443318916f Mon Sep 17 00:00:00 2001 From: Sebastian Stehle Date: Tue, 23 Jan 2018 10:37:15 +0100 Subject: [PATCH] Improved validation handling. --- .../Contents/MongoContentRepository.cs | 28 ++++++--- .../ValidationException.cs | 59 +++++++++++++++---- .../Pipeline/ApiExceptionFilterAttribute.cs | 2 +- .../ValidationExceptionTests.cs | 54 +++++++++++++++++ 4 files changed, 122 insertions(+), 21 deletions(-) create mode 100644 tests/Squidex.Infrastructure.Tests/ValidationExceptionTests.cs diff --git a/src/Squidex.Domain.Apps.Entities.MongoDb/Contents/MongoContentRepository.cs b/src/Squidex.Domain.Apps.Entities.MongoDb/Contents/MongoContentRepository.cs index e7aff13a2..36841edfa 100644 --- a/src/Squidex.Domain.Apps.Entities.MongoDb/Contents/MongoContentRepository.cs +++ b/src/Squidex.Domain.Apps.Entities.MongoDb/Contents/MongoContentRepository.cs @@ -87,17 +87,31 @@ namespace Squidex.Domain.Apps.Entities.MongoDb.Contents throw new ValidationException("This odata operation is not supported."); } - var contentItems = Collection.Find(filter).Take(odataQuery).Skip(odataQuery).Sort(odataQuery, schema.SchemaDef).ToListAsync(); - var contentCount = Collection.Find(filter).CountAsync(); + try + { + var contentItems = Collection.Find(filter).Take(odataQuery).Skip(odataQuery).Sort(odataQuery, schema.SchemaDef).ToListAsync(); + var contentCount = Collection.Find(filter).CountAsync(); - await Task.WhenAll(contentItems, contentCount); + await Task.WhenAll(contentItems, contentCount); - foreach (var entity in contentItems.Result) + foreach (var entity in contentItems.Result) + { + entity.ParseData(schema.SchemaDef); + } + + return ResultList.Create(contentItems.Result, contentCount.Result); + } + catch (MongoQueryException ex) { - entity.ParseData(schema.SchemaDef); + if (ex.Message.Contains("17406")) + { + throw new DomainException("Result set is too large to be retrieved. Use $top parameter to reduce the number of items."); + } + else + { + throw; + } } - - return ResultList.Create(contentItems.Result, contentCount.Result); } public async Task> QueryAsync(IAppEntity app, ISchemaEntity schema, Status[] status, HashSet ids) diff --git a/src/Squidex.Infrastructure/ValidationException.cs b/src/Squidex.Infrastructure/ValidationException.cs index 29bf22532..163904187 100644 --- a/src/Squidex.Infrastructure/ValidationException.cs +++ b/src/Squidex.Infrastructure/ValidationException.cs @@ -9,11 +9,12 @@ using System; using System.Collections.Generic; using System.Linq; using System.Runtime.Serialization; +using System.Text; namespace Squidex.Infrastructure { [Serializable] - public class ValidationException : Exception + public class ValidationException : DomainException { private static readonly List FallbackErrors = new List(); private readonly IReadOnlyList errors; @@ -23,27 +24,29 @@ namespace Squidex.Infrastructure get { return errors; } } - public ValidationException(string message, params ValidationError[] errors) - : base(message) + public string Summary { get; } + + public ValidationException(string summary, params ValidationError[] errors) + : this(summary, null, errors?.ToList()) { - this.errors = errors != null ? errors.ToList() : FallbackErrors; } - public ValidationException(string message, IReadOnlyList errors) - : base(message) + public ValidationException(string summary, IReadOnlyList errors) + : this(summary, null, errors) { this.errors = errors ?? FallbackErrors; } - public ValidationException(string message, Exception inner, params ValidationError[] errors) - : base(message, inner) + public ValidationException(string summary, Exception inner, params ValidationError[] errors) + : this(summary, null, errors?.ToList()) { - this.errors = errors != null ? errors.ToList() : FallbackErrors; } - public ValidationException(string message, Exception inner, IReadOnlyList errors) - : base(message, inner) + public ValidationException(string summary, Exception inner, IReadOnlyList errors) + : base(FormatMessage(summary, errors), inner) { + Summary = summary; + this.errors = errors ?? FallbackErrors; } @@ -52,9 +55,39 @@ namespace Squidex.Infrastructure { } - public override string ToString() + private static string FormatMessage(string summary, IReadOnlyList errors) { - return string.Join(" ", Enumerable.Repeat(Message, 1).Union(Errors.Select(x => x.Message))); + var sb = new StringBuilder(); + + sb.Append(summary.TrimEnd(' ', '.', ':')); + + if (errors?.Count > 0) + { + sb.Append(": "); + + for (var i = 0; i < errors.Count; i++) + { + var error = errors[i].Message; + + sb.Append(error); + + if (!error.EndsWith(".", StringComparison.OrdinalIgnoreCase)) + { + sb.Append("."); + } + + if (i < errors.Count - 1) + { + sb.Append(" "); + } + } + } + else + { + sb.Append("."); + } + + return sb.ToString(); } } } diff --git a/src/Squidex/Pipeline/ApiExceptionFilterAttribute.cs b/src/Squidex/Pipeline/ApiExceptionFilterAttribute.cs index a8f2adc05..505490175 100644 --- a/src/Squidex/Pipeline/ApiExceptionFilterAttribute.cs +++ b/src/Squidex/Pipeline/ApiExceptionFilterAttribute.cs @@ -54,7 +54,7 @@ namespace Squidex.Pipeline private static IActionResult OnValidationException(ValidationException ex) { - return ErrorResult(400, new ErrorDto { Message = ex.Message, Details = ex.Errors.Select(e => e.Message).ToArray() }); + return ErrorResult(400, new ErrorDto { Message = ex.Summary, Details = ex.Errors.Select(e => e.Message).ToArray() }); } private static IActionResult ErrorResult(int statusCode, ErrorDto error) diff --git a/tests/Squidex.Infrastructure.Tests/ValidationExceptionTests.cs b/tests/Squidex.Infrastructure.Tests/ValidationExceptionTests.cs new file mode 100644 index 000000000..10e19bcee --- /dev/null +++ b/tests/Squidex.Infrastructure.Tests/ValidationExceptionTests.cs @@ -0,0 +1,54 @@ +// ========================================================================== +// Squidex Headless CMS +// ========================================================================== +// Copyright (c) Squidex UG (haftungsbeschraenkt) +// All rights reserved. Licensed under the MIT license. +// ========================================================================== + +using Xunit; + +namespace Squidex.Infrastructure +{ + public class ValidationExceptionTests + { + [Fact] + public void Should_format_message_from_summary() + { + var ex = new ValidationException("Summary."); + + Assert.Equal("Summary.", ex.Message); + } + + [Fact] + public void Should_append_dot_to_summary() + { + var ex = new ValidationException("Summary"); + + Assert.Equal("Summary.", ex.Message); + } + + [Fact] + public void Should_format_message_from_errors() + { + var ex = new ValidationException("Summary", new ValidationError("Error1."), new ValidationError("Error2.")); + + Assert.Equal("Summary: Error1. Error2.", ex.Message); + } + + [Fact] + public void Should_not_add_colon_twice() + { + var ex = new ValidationException("Summary:", new ValidationError("Error1."), new ValidationError("Error2.")); + + Assert.Equal("Summary: Error1. Error2.", ex.Message); + } + + [Fact] + public void Should_append_dots_to_errors() + { + var ex = new ValidationException("Summary", new ValidationError("Error1"), new ValidationError("Error2")); + + Assert.Equal("Summary: Error1. Error2.", ex.Message); + } + } +}