Merge from master

ofpinewood · Dec 11, 2020 · 084cc07 · 084cc07
2 parents 9c7e2f7 + 9470805
commit 084cc07
Show file tree

Hide file tree

Showing 12 changed files with 203 additions and 3 deletions.
diff --git a/README.md b/README.md
@@ -55,10 +55,32 @@ Whether or not to include the full exception details in the response. The defaul
 mvcBuilder.AddHttpExceptions(options =>
 {
     // This is the same as the default behavior; only include exception details in a development environment.
-    options.IncludeExceptionDetails= context => context.RequestServices.GetRequiredService<IHostingEnvironment>().IsDevelopment();
+    options.IncludeExceptionDetails = context => context.RequestServices.GetRequiredService<IHostingEnvironment>().IsDevelopment();
 });
 ```
 
+#### Include extra properties on custom exceptions in the exception details
+You can include extra public exception properties in exception details, by adding the `ProblemDetailsAttribute` to them.
+
+``` csharp
+mvcBuilder.AddHttpExceptions(options =>
+{
+    // The default is also true.
+    options.IsProblemDetailsAttributeEnabled true;
+});
+
+public class CustomHttpException : HttpException
+{
+    [ProblemDetails]
+    public string PropertyA { get; set; }
+
+    [ProblemDetails]
+    public int PropertyB { get; set; }
+
+    public long PropertyC { get; set; }
+}
+```
+
 #### Is exception response
 Is the response an exception and should it be handled by the HttpExceptions middleware.
 

diff --git a/samples/Opw.HttpExceptions.AspNetCore.Sample/Controllers/TestController.cs b/samples/Opw.HttpExceptions.AspNetCore.Sample/Controllers/TestController.cs
@@ -1,5 +1,6 @@
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Mvc;
+using Opw.HttpExceptions.AspNetCore.Sample.CustomErrors;
 using Opw.HttpExceptions.AspNetCore.Sample.Models;
 using System;
 using System.Net;
@@ -18,6 +19,12 @@ public ActionResult<string> Throw(HttpStatusCode statusCode)
             throw new HttpException(statusCode, message, innerException);
         }
 
+        [HttpGet("problemDetailsAttributeException")]
+        public ActionResult<string> ThrowProblemDetailsAttributeException()
+        {
+            throw new ProblemDetailsAttributeException("ProblemDetailsAttributeException has occurred.") { PropertyA = "AAA", PropertyB = 42, PropertyC = 123L };
+        }
+
         [HttpGet("applicationException")]
         public ActionResult<string> ThrowApplicationException()
         {

diff --git a/...les/Opw.HttpExceptions.AspNetCore.Sample/CustomErrors/ProblemDetailsAttributeException.cs b/...les/Opw.HttpExceptions.AspNetCore.Sample/CustomErrors/ProblemDetailsAttributeException.cs
@@ -0,0 +1,23 @@
+using System;
+using System.Net;
+using System.Runtime.Serialization;
+
+namespace Opw.HttpExceptions.AspNetCore.Sample.CustomErrors
+{
+    public class ProblemDetailsAttributeException : HttpException
+    {
+        public ProblemDetailsAttributeException(string message) : base(HttpStatusCode.Ambiguous, message) { }
+
+        public ProblemDetailsAttributeException(string message, Exception innerException) : base(message, innerException) { }
+
+        public ProblemDetailsAttributeException(SerializationInfo info, StreamingContext context) : base(info, context) { }
+
+        [ProblemDetails]
+        public string PropertyA { get; set; }
+
+        [ProblemDetails]
+        public int PropertyB { get; set; }
+
+        public long PropertyC { get; set; }
+    }
+}
diff --git a/src/Opw.HttpExceptions.AspNetCore/HttpExceptionsOptions.cs b/src/Opw.HttpExceptions.AspNetCore/HttpExceptionsOptions.cs
@@ -16,6 +16,12 @@ public class HttpExceptionsOptions
         /// </summary>
         public Func<HttpContext, bool> IncludeExceptionDetails { get; set; }
 
+        /// <summary>
+        /// Include all public exception properties that are attributed with <see cref="ProblemDetailsAttribute"/> in exception details.
+        /// Default <c>true</c>
+        /// </summary>
+        public bool IsProblemDetailsAttributeEnabled { get; set; } = true;
+
         /// <summary>
         /// Is the response an exception and should it be handled by the HttpExceptions middleware.
         /// </summary>

diff --git a/src/Opw.HttpExceptions.AspNetCore/InternalsVisibleTo.cs b/src/Opw.HttpExceptions.AspNetCore/InternalsVisibleTo.cs
@@ -1,2 +1,3 @@
 using System.Runtime.CompilerServices;
 [assembly: InternalsVisibleTo("Opw.HttpExceptions.AspNetCore.Tests")]
+[assembly: InternalsVisibleTo("Opw.HttpExceptions.AspNetCore.Sample.Tests")]
diff --git a/src/Opw.HttpExceptions.AspNetCore/Mappers/ProblemDetailsExceptionMapper.cs b/src/Opw.HttpExceptions.AspNetCore/Mappers/ProblemDetailsExceptionMapper.cs
@@ -3,6 +3,7 @@
 using Microsoft.AspNetCore.Mvc.Infrastructure;
 using Microsoft.Extensions.Options;
 using System;
+using System.Linq;
 using System.Net;
 
 namespace Opw.HttpExceptions.AspNetCore.Mappers
@@ -84,6 +85,18 @@ public virtual IStatusCodeActionResult Map(Exception exception, HttpContext cont
             if (Options.Value.IncludeExceptionDetails(context))
                 problemDetails.Extensions.Add(nameof(ProblemDetailsExtensionMembers.ExceptionDetails).ToCamelCase(), new SerializableException(exception));
 
+            if (Options.Value.IsProblemDetailsAttributeEnabled)
+            {
+                foreach (var propertyInfo in exception.GetType().GetProperties()
+                    .Where(p => p.GetCustomAttributes(typeof(ProblemDetailsAttribute), true)?.Any() == true))
+                {
+                    if (propertyInfo.CanRead)
+                    {
+                        problemDetails.Extensions.Add(propertyInfo.Name.ToCamelCase(), propertyInfo.GetValue(exception));
+                    }
+                }
+            }
+
             return new ProblemDetailsResult(problemDetails);
         }
 

diff --git a/src/Opw.HttpExceptions/ProblemDetailsAttribute.cs b/src/Opw.HttpExceptions/ProblemDetailsAttribute.cs
@@ -0,0 +1,12 @@
+using System;
+
+namespace Opw.HttpExceptions
+{
+    /// <summary>
+    /// Include the property in the problem details in exception details.
+    /// </summary>
+    [AttributeUsage(AttributeTargets.Property, Inherited = true, AllowMultiple = false)]
+    public class ProblemDetailsAttribute : Attribute
+    {
+    }
+}
diff --git a/src/Opw.HttpExceptions/SerializableException.cs b/src/Opw.HttpExceptions/SerializableException.cs
@@ -1,5 +1,4 @@
 using System;
-using System.Collections;
 using System.Collections.Generic;
 using System.Linq;
 using System.Runtime.Serialization;
@@ -85,7 +84,7 @@ public SerializableException(Exception exception)
                 nameof(exception.TargetSite)
             };
 
-            foreach (var propertyInfo in exception.GetType().GetProperties())
+            foreach (var propertyInfo in exception.GetType().GetProperties())//.Where(p => p.GetCustomAttributes(typeof(ProblemDetailsAttribute), true)?.Any() != true))
             {
                 if (propertiesToExclude.Any(p => p == propertyInfo.Name)) continue;
                 if (propertyInfo.CanRead)

diff --git a/tests/Opw.HttpExceptions.AspNetCore.Sample.Tests/Controllers/HttpExceptionControllerTests.cs b/tests/Opw.HttpExceptions.AspNetCore.Sample.Tests/Controllers/HttpExceptionControllerTests.cs
@@ -70,6 +70,20 @@ public async Task ThrowApplicationException_Should_ReturnProblemDetails()
             problemDetails.Extensions.Should().HaveCount(0);
         }
 
+        [Fact]
+        public async Task ThrowProblemDetailsAttributeException_Should_ReturnProblemDetails()
+        {
+            var response = await _client.GetAsync("test/problemDetailsAttributeException");
+
+            var problemDetails = response.ShouldBeProblemDetails(HttpStatusCode.Ambiguous);
+            problemDetails.Title.Should().Be("ProblemDetailsAttribute");
+            problemDetails.Type.Should().Be("error:problem-details-attribute");
+            problemDetails.Extensions.Should().HaveCount(2);
+            problemDetails.Extensions[nameof(ProblemDetailsAttributeException.PropertyA).ToCamelCase()].ToString().Should().Be("AAA");
+            problemDetails.Extensions[nameof(ProblemDetailsAttributeException.PropertyB).ToCamelCase()].ToString().Should().Be("42");
+            problemDetails.Extensions.Should().NotContainKey(nameof(ProblemDetailsAttributeException.PropertyC).ToCamelCase());
+        }
+
         [Fact]
         public async Task PostProduct_Should_ReturnOk()
         {

diff --git a/tests/Opw.HttpExceptions.AspNetCore.Tests/Mappers/ProblemDetailsExceptionMapperTests.cs b/tests/Opw.HttpExceptions.AspNetCore.Tests/Mappers/ProblemDetailsExceptionMapperTests.cs
@@ -1,6 +1,7 @@
 using FluentAssertions;
 using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.Options;
+using Opw.HttpExceptions.AspNetCore.Tests.TestData;
 using System;
 using System.Collections.Generic;
 using System.ComponentModel.DataAnnotations;
@@ -151,6 +152,28 @@ public void Map_Should_ReturnProblemDetails_WithValidationErrors2_ForValidationE
             errors.Should().HaveCount(2);
         }
 
+        [Fact]
+        public void Map_Should_ReturnPropertiesAsProblemDetailsExtensions_ForExceptionWithProblemDetailsAttributeProperties()
+        {
+            var actionResult = _mapper.Map(new ProblemDetailsAttributeException("ProblemDetailsAttributeException has occurred.") { PropertyA = "AAA", PropertyB = 42, PropertyC = 123L }, new DefaultHttpContext());
+
+            actionResult.Should().BeOfType<ProblemDetailsResult>();
+            var problemDetailsResult = (ProblemDetailsResult)actionResult;
+
+            problemDetailsResult.Value.ShouldNotBeNull(HttpStatusCode.Ambiguous);
+            problemDetailsResult.Value.Instance.Should().NotBeNull();
+
+            problemDetailsResult.Value.Extensions.Should().HaveCount(2);
+            problemDetailsResult.Value.Extensions[nameof(ProblemDetailsAttributeException.PropertyA).ToCamelCase()].ToString().Should().Be("AAA");
+            problemDetailsResult.Value.Extensions[nameof(ProblemDetailsAttributeException.PropertyB).ToCamelCase()].ToString().Should().Be("42");
+            problemDetailsResult.Value.Extensions.Should().NotContainKey(nameof(ProblemDetailsAttributeException.PropertyC).ToCamelCase());
+
+            var result = problemDetailsResult.Value.TryGetExceptionDetails(out var exception);
+
+            result.Should().BeFalse();
+            exception.Should().BeNull();
+        }
+
         [Fact]
         public void MapDetail_Should_ReturnDetail_UsingOptionsExceptionDetailMappingOverride()
         {

diff --git a/tests/Opw.HttpExceptions.AspNetCore.Tests/TestData/ProblemDetailsAttributeException.cs b/tests/Opw.HttpExceptions.AspNetCore.Tests/TestData/ProblemDetailsAttributeException.cs
@@ -0,0 +1,23 @@
+using System;
+using System.Net;
+using System.Runtime.Serialization;
+
+namespace Opw.HttpExceptions.AspNetCore.Tests.TestData
+{
+    public class ProblemDetailsAttributeException : HttpException
+    {
+        public ProblemDetailsAttributeException(string message) : base(HttpStatusCode.Ambiguous, message) { }
+
+        public ProblemDetailsAttributeException(string message, Exception innerException) : base(message, innerException) { }
+
+        public ProblemDetailsAttributeException(SerializationInfo info, StreamingContext context) : base(info, context) { }
+
+        [ProblemDetails]
+        public string PropertyA { get; set; }
+
+        [ProblemDetails]
+        public int PropertyB { get; set; }
+
+        public long PropertyC { get; set; }
+    }
+}
diff --git a/tests/Opw.HttpExceptions.Tests/ProblemDetailAttributeTests.cs b/tests/Opw.HttpExceptions.Tests/ProblemDetailAttributeTests.cs
@@ -0,0 +1,57 @@
+using FluentAssertions;
+using Xunit;
+
+namespace Opw.HttpExceptions
+{
+    public class ProblemDetailAttributeTests
+    {
+        [Fact]
+        public void Property_Should_HaveAttribute()
+        {
+            new Person().GetType().GetProperty(nameof(Person.Id))
+                .GetCustomAttributes(false).Should()
+                .Contain(new ProblemDetailsAttribute());
+        }
+
+        [Fact]
+        public void Customer_BaseProperty_Should_HaveAttribute()
+        {
+            new Customer().GetType().GetProperty(nameof(Person.Name))
+                .GetCustomAttributes(false).Should()
+                .Contain(new ProblemDetailsAttribute());
+        }
+
+        [Fact]
+        public void Customer_OverriddenBaseProperty_Should_HaveAttribute()
+        {
+            new Customer().GetType().GetProperty(nameof(Customer.Id))
+                .GetCustomAttributes(false).Should()
+                .Contain(new ProblemDetailsAttribute());
+        }
+
+        [Fact]
+        public void Customer_Property_Should_NotHaveAttribute()
+        {
+            new Customer().GetType().GetProperty(nameof(Customer.Code))
+                .GetCustomAttributes(false).Should()
+                .NotContain(new ProblemDetailsAttribute());
+        }
+
+        private class Person
+        {
+            [ProblemDetails]
+            public virtual int Id { get; set; }
+
+            [ProblemDetails]
+            public string Name { get; set; }
+        }
+
+        private class Customer : Person
+        {
+            [ProblemDetails]
+            public override int Id { get; set; }
+
+            public string Code { get; set; }
+        }
+    }
+}