Fix 179 (#180)

* Working on fix for 179 * merge * Fix build warnings * Fixing tests Added fix for 179 Added test to confirm fix
ardalis · May 15, 2024 · 54b5de8 · 54b5de8
1 parent ddbac3f
commit 54b5de8
Show file tree

Hide file tree

Showing 21 changed files with 149 additions and 39 deletions.
diff --git a/sample/Ardalis.Result.Sample.Core/Ardalis.Result.Sample.Core.csproj b/sample/Ardalis.Result.Sample.Core/Ardalis.Result.Sample.Core.csproj
@@ -1,19 +1,18 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
-  <PropertyGroup>
-    <AssemblyName>Ardalis.Result.Sample.Core</AssemblyName>
-    <RootNamespace>Ardalis.Result.Sample.Core</RootNamespace>
-  </PropertyGroup>
+	<PropertyGroup>
+		<AssemblyName>Ardalis.Result.Sample.Core</AssemblyName>
+		<RootNamespace>Ardalis.Result.Sample.Core</RootNamespace>
+	</PropertyGroup>
 
-  <ItemGroup>
-    <PackageReference Include="Microsoft.Extensions.Localization.Abstractions" />
-    <PackageReference Include="System.ComponentModel.Annotations" />
-    <PackageReference Include="System.Runtime" />
-  </ItemGroup>
-
-  <ItemGroup>
-    <ProjectReference Include="..\..\src\Ardalis.Result.FluentValidation\Ardalis.Result.FluentValidation.csproj" />
-    <ProjectReference Include="..\..\src\Ardalis.Result\Ardalis.Result.csproj" />
-  </ItemGroup>
+	<ItemGroup>
+		<PackageReference Include="Microsoft.Extensions.Localization.Abstractions" />
+		<PackageReference Include="System.ComponentModel.Annotations" />
+		<PackageReference Include="System.Runtime" />
+	</ItemGroup>
 
+	<ItemGroup>
+		<ProjectReference Include="..\..\src\Ardalis.Result.FluentValidation\Ardalis.Result.FluentValidation.csproj" />
+		<ProjectReference Include="..\..\src\Ardalis.Result\Ardalis.Result.csproj" />
+	</ItemGroup>
 </Project>
diff --git a/sample/Ardalis.Result.Sample.Core/DTOs/CreatePersonRequestDto.cs b/sample/Ardalis.Result.Sample.Core/DTOs/CreatePersonRequestDto.cs
@@ -1,7 +1,9 @@
-namespace Ardalis.Result.Sample.Core.DTOs;
+using System;
+
+namespace Ardalis.Result.Sample.Core.DTOs;
 
 public class CreatePersonRequestDto
 {
-    public string FirstName { get; set; }  
-    public string LastName { get; set; }  
+    public string FirstName { get; set; } = String.Empty; 
+    public string LastName { get; set; } = String.Empty;  
 }
diff --git a/sample/Ardalis.Result.Sample.Core/DTOs/ForecastRequestDto.cs b/sample/Ardalis.Result.Sample.Core/DTOs/ForecastRequestDto.cs
@@ -1,11 +1,12 @@
 
+using System;
 using System.ComponentModel.DataAnnotations;
 
 namespace Ardalis.Result.Sample.Core.DTOs
 { 
     public class ForecastRequestDto
     {
         [Required]
-        public string PostalCode { get; set; }
+        public string PostalCode { get; set; } = String.Empty;
     }
 }
diff --git a/sample/Ardalis.Result.Sample.Core/Exceptions/ForecastRequestInvalidException.cs b/sample/Ardalis.Result.Sample.Core/Exceptions/ForecastRequestInvalidException.cs
@@ -5,7 +5,7 @@ namespace Ardalis.Result.Sample.Core.Exceptions
 {
     public class ForecastRequestInvalidException : Exception
     {
-        public Dictionary<string,string> ValidationErrors { get; set; }
+        public Dictionary<string,string> ValidationErrors { get; set; } = new();
 
         public ForecastRequestInvalidException(Dictionary<string, string> validationErrors) : base("Forecast request is invalid.")
         {

diff --git a/sample/Ardalis.Result.Sample.Core/Model/Person.cs b/sample/Ardalis.Result.Sample.Core/Model/Person.cs
@@ -6,10 +6,10 @@ namespace Ardalis.Result.Sample.Core.Model
     public class Person
     {
 		public int Id { get; set; }
-		public string Surname { get; set; }
-		public string Forename { get; set; }
+		public string Surname { get; set; } = String.Empty;
+		public string Forename { get; set; } = String.Empty;
 
-		public List<Person> Children { get; set; }
+		public List<Person> Children { get; set; } = new();
 
 		public DateTime DateOfBirth { get; set; }
 	}

diff --git a/sample/Ardalis.Result.Sample.Core/Model/WeatherForecast.cs b/sample/Ardalis.Result.Sample.Core/Model/WeatherForecast.cs
@@ -10,6 +10,6 @@ public class WeatherForecast
 
         public int TemperatureF => 32 + (int)(TemperatureC / 0.5556);
 
-        public string Summary { get; set; }
+        public string Summary { get; set; } = String.Empty;
     }
 }
diff --git a/sample/Ardalis.Result.SampleMinimalApi.FunctionalTests/NewWeatherForecast.cs b/sample/Ardalis.Result.SampleMinimalApi.FunctionalTests/NewWeatherForecast.cs
@@ -33,7 +33,7 @@ public async Task ReturnsOkWithValueGivenValidPostalCode()
         var stringResponse = await response.Content.ReadAsStringAsync();
         var forecasts = JsonConvert.DeserializeObject<List<WeatherForecast>>(stringResponse);
 
-        Assert.Equal("Freezing", forecasts.First().Summary);
+        Assert.Equal("Freezing", forecasts?.First()?.Summary);
     }
 
     [Fact]

diff --git a/sample/Ardalis.Result.SampleWeb.FunctionalTests/WeatherForecastControllerPost.cs b/sample/Ardalis.Result.SampleWeb.FunctionalTests/WeatherForecastControllerPost.cs
@@ -13,6 +13,30 @@
 
 namespace Ardalis.Result.SampleWeb.FunctionalTests;
 
+public class WeatherForecastControllerThrows : IClassFixture<WebApplicationFactory<WebMarker>>
+{
+    private const string CONTROLLER_THROWS_ROUTE = "/weatherforecast/throws";
+    private readonly HttpClient _client;
+
+    public WeatherForecastControllerThrows(WebApplicationFactory<WebMarker> factory)
+    {
+        _client = factory.CreateClient();
+    }
+
+    [Fact]
+    public async Task Returns400BadRequestNot500()
+    {
+        var response = await _client.GetAsync(CONTROLLER_THROWS_ROUTE);
+
+        Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
+        var stringResponse = await response.Content.ReadAsStringAsync();
+        var problemDetails = JsonConvert.DeserializeObject<ProblemDetails>(stringResponse);
+
+        Assert.Equal("One or more validation errors occurred.", problemDetails?.Title);
+        Assert.Equal(400, problemDetails.Status);
+    }
+}
+
 public class WeatherForecastControllerPost : IClassFixture<WebApplicationFactory<WebMarker>>
 {
     private const string CONTROLLER_POST_ROUTE = "/weatherforecast/create";
@@ -36,7 +60,7 @@ public async Task ReturnsOkWithValueGivenValidPostalCode(string route)
         var stringResponse = await response.Content.ReadAsStringAsync();
         var forecasts = JsonConvert.DeserializeObject<List<WeatherForecast>>(stringResponse);
 
-        Assert.Equal("Freezing", forecasts.First().Summary);
+        Assert.Equal("Freezing", forecasts?.First()?.Summary);
     }
 
     [Theory]

diff --git a/sample/Ardalis.Result.SampleWeb/MediatrApi/WeatherForecastController.cs b/sample/Ardalis.Result.SampleWeb/MediatrApi/WeatherForecastController.cs
@@ -46,7 +46,7 @@ public Task<Result<IEnumerable<WeatherForecast>>> CreateForecast([FromBody] NewF
     public class NewForecastCommand : IRequest<Result<IEnumerable<WeatherForecast>>>
     {
         [Required]
-        public string PostalCode { get; set; }
+        public string PostalCode { get; set; } = String.Empty;
     }
 
     public class NewForecastHandler : IRequestHandler<NewForecastCommand, Result<IEnumerable<WeatherForecast>>>

diff --git a/sample/Ardalis.Result.SampleWeb/Pages/Index.cshtml.cs b/sample/Ardalis.Result.SampleWeb/Pages/Index.cshtml.cs
@@ -12,7 +12,7 @@ public IndexModel(IStringLocalizer<IndexModel> stringLocalizer)
         _stringLocalizer = stringLocalizer;
     }
 
-    public string Message { get; set; }
+    public string Message { get; set; } = String.Empty;
     public void OnGet()
     {
         Message = _stringLocalizer["message"].Value;

diff --git a/sample/Ardalis.Result.SampleWeb/WeatherForecastFeature/WeatherForecastController.cs b/sample/Ardalis.Result.SampleWeb/WeatherForecastFeature/WeatherForecastController.cs
@@ -50,4 +50,15 @@ public ActionResult<WeatherForecastSummaryDto> CreateSummaryForecast([FromBody]
             // .Map(WeatherForecastSummaryDto.MapFrom)
             .ToActionResult(this);
     }
+
+    /// <summary>
+    /// Issue #179
+    /// </summary>
+    /// <returns></returns>
+    [HttpGet("throws")]
+    [TranslateResultToActionResult]
+    public Result<int> Throws()
+    {
+        return Result<int>.Invalid(new ValidationError("foo"));
+    }
 }
diff --git a/sample/Directory.Build.props b/sample/Directory.Build.props
@@ -1,7 +1,7 @@
 <Project>
   <PropertyGroup>
     <NetCoreFrameworks>net6.0;net7.0;net8.0</NetCoreFrameworks>
-    <TargetFrameworks>net4.7.1;$(NetCoreFrameworks)</TargetFrameworks>
+    <TargetFrameworks>net48;$(NetCoreFrameworks)</TargetFrameworks>
     <Nullable>enable</Nullable>
     <LangVersion>latest</LangVersion>
   </PropertyGroup>

diff --git a/src/Ardalis.Result.AspNetCore/ResultStatusMap.cs b/src/Ardalis.Result.AspNetCore/ResultStatusMap.cs
@@ -102,7 +102,11 @@ private static ValidationProblemDetails BadRequest(ControllerBase controller, IR
         {
             foreach (var error in result.ValidationErrors)
             {
-                controller.ModelState.AddModelError(error.Identifier, error.ErrorMessage);
+                // TODO: mark ValidationError.Identifier as required and limit setting (see #179)
+                string identifier = error.Identifier ?? "(identifier)";
+                // TODO: mark ValidationError.Identifier as required and limit setting (see #179)
+                string errorMessage = error.ErrorMessage ?? "(error message)";
+                controller.ModelState.AddModelError(identifier, errorMessage);
             }
 
             return new ValidationProblemDetails(controller.ModelState);

diff --git a/src/Ardalis.Result/ValidationError.cs b/src/Ardalis.Result/ValidationError.cs
@@ -20,6 +20,7 @@ public ValidationError(string identifier, string errorMessage, string errorCode,
         }
 
         public string Identifier { get; set; }
+        // TODO: Mark required and limit setting (see #179)
         public string ErrorMessage { get; set; }
         public string ErrorCode { get; set; }
         public ValidationSeverity Severity { get; set; } = ValidationSeverity.Error;

diff --git a/src/Directory.Build.props b/src/Directory.Build.props
@@ -1,7 +1,7 @@
 <Project>
   <PropertyGroup>
     <NetCoreFrameworks>net6.0;net7.0;net8.0</NetCoreFrameworks>
-    <TargetFrameworks>netstandard2.0</TargetFrameworks>
+     <TargetFrameworks>netstandard2.0;$(NetCoreFrameworks)</TargetFrameworks>
     <LangVersion>latest</LangVersion>
     <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
     <Authors>Steve Smith (@ardalis); Shady Nagy (@ShadyNagy)</Authors>

diff --git a/tests/Ardalis.Result.AspNetCore.UnitTests/BaseResultConventionTest.cs b/tests/Ardalis.Result.AspNetCore.UnitTests/BaseResultConventionTest.cs
@@ -4,7 +4,6 @@
 using System.Reflection;
 
 namespace Ardalis.Result.AspNetCore.UnitTests;
-
 public class BaseResultConventionTest
 {
     protected bool ProducesResponseTypeAttribute(IFilterMetadata filterMetadata, int statusCode, Type type)

diff --git a/tests/Ardalis.Result.AspNetCore.UnitTests/ResultConventionDefaultResultStatusMap.cs b/tests/Ardalis.Result.AspNetCore.UnitTests/ResultConventionDefaultResultStatusMap.cs
@@ -110,9 +110,10 @@ public void ResultWithValue(string actionName, Type expectedType)
 
         convention.Apply(actionModel);
 
-        Assert.Equal(9, actionModel.Filters.Where(f => f is ProducesResponseTypeAttribute).Count());
+        Assert.Equal(10, actionModel.Filters.Where(f => f is ProducesResponseTypeAttribute).Count());
 
         Assert.Contains(actionModel.Filters, f => ProducesResponseTypeAttribute(f, 200, expectedType));
+        Assert.Contains(actionModel.Filters, f => ProducesResponseTypeAttribute(f, 204, typeof(void)));
         Assert.Contains(actionModel.Filters, f => ProducesResponseTypeAttribute(f, 404, typeof(ProblemDetails)));
         Assert.Contains(actionModel.Filters, f => ProducesResponseTypeAttribute(f, 400, typeof(ValidationProblemDetails)));
         Assert.Contains(actionModel.Filters, f => ProducesResponseTypeAttribute(f, 401, typeof(void)));

diff --git a/tests/Ardalis.Result.AspNetCore.UnitTests/ResultConventionDefaultResultStatusMapModified.cs b/tests/Ardalis.Result.AspNetCore.UnitTests/ResultConventionDefaultResultStatusMapModified.cs
@@ -62,12 +62,8 @@ public void ChangeResultStatus()
     [InlineData("Index", typeof(void), typeof(HttpPostAttribute), 204)]
     [InlineData("Index", typeof(void), typeof(HttpDeleteAttribute), 204)]
     [InlineData("Index", typeof(void), typeof(HttpGetAttribute), 204)]
-    [InlineData(nameof(TestController.ResultString), typeof(string), typeof(HttpPostAttribute), 201)]
     [InlineData(nameof(TestController.ResultString), typeof(string), typeof(HttpDeleteAttribute), 204)]
-    [InlineData(nameof(TestController.ResultString), typeof(string), typeof(HttpGetAttribute), 200)]
-    [InlineData(nameof(TestController.ResultEnumerableString), typeof(IEnumerable<string>), typeof(HttpPostAttribute), 201)]
     [InlineData(nameof(TestController.ResultEnumerableString), typeof(IEnumerable<string>), typeof(HttpDeleteAttribute), 204)]
-    [InlineData(nameof(TestController.ResultEnumerableString), typeof(IEnumerable<string>), typeof(HttpGetAttribute), 200)]
     public void ChangeResultStatus_ForSpecificMethods(string actionName, Type expectedType, Type attributeType, int expectedStatusCode)
     {
         var convention = new ResultConvention(new ResultStatusMap()
@@ -96,4 +92,39 @@ public void ChangeResultStatus_ForSpecificMethods(string actionName, Type expect
         Assert.Contains(actionModel.Filters, f => ProducesResponseTypeAttribute(f, 500, typeof(ProblemDetails)));
         Assert.Contains(actionModel.Filters, f => ProducesResponseTypeAttribute(f, 503, typeof(ProblemDetails)));
     }
+
+    [Theory]
+    [InlineData(nameof(TestController.ResultString), typeof(string), typeof(HttpPostAttribute), 201)]
+    [InlineData(nameof(TestController.ResultString), typeof(string), typeof(HttpGetAttribute), 200)]
+    [InlineData(nameof(TestController.ResultEnumerableString), typeof(IEnumerable<string>), typeof(HttpPostAttribute), 201)]
+    [InlineData(nameof(TestController.ResultEnumerableString), typeof(IEnumerable<string>), typeof(HttpGetAttribute), 200)]
+    public void ChangeResultStatus_ForDeleteMethods(string actionName, Type expectedType, Type attributeType, int expectedStatusCode)
+    {
+        var convention = new ResultConvention(new ResultStatusMap()
+            .AddDefaultMap()
+            .For(ResultStatus.Ok, HttpStatusCode.OK, opts => opts
+                .For("POST", HttpStatusCode.Created)
+                .For("DELETE", HttpStatusCode.NoContent)));
+
+        var actionModelBuilder = new ActionModelBuilder()
+            .AddActionFilter(new TranslateResultToActionResultAttribute())
+            .AddActionAttribute((Attribute)Activator.CreateInstance(attributeType)!);
+
+        var actionModel = actionModelBuilder.GetActionModel(actionName);
+
+        convention.Apply(actionModel);
+
+        Assert.Equal(10, actionModel.Filters.Where(f => f is ProducesResponseTypeAttribute).Count());
+
+        Assert.Contains(actionModel.Filters, f => ProducesResponseTypeAttribute(f, expectedStatusCode, expectedType));
+        Assert.Contains(actionModel.Filters, f => ProducesResponseTypeAttribute(f, 204, typeof(void)));
+        Assert.Contains(actionModel.Filters, f => ProducesResponseTypeAttribute(f, 404, typeof(ProblemDetails)));
+        Assert.Contains(actionModel.Filters, f => ProducesResponseTypeAttribute(f, 400, typeof(ValidationProblemDetails)));
+        Assert.Contains(actionModel.Filters, f => ProducesResponseTypeAttribute(f, 401, typeof(void)));
+        Assert.Contains(actionModel.Filters, f => ProducesResponseTypeAttribute(f, 403, typeof(void)));
+        Assert.Contains(actionModel.Filters, f => ProducesResponseTypeAttribute(f, 409, typeof(ProblemDetails)));
+        Assert.Contains(actionModel.Filters, f => ProducesResponseTypeAttribute(f, 422, typeof(ProblemDetails)));
+        Assert.Contains(actionModel.Filters, f => ProducesResponseTypeAttribute(f, 500, typeof(ProblemDetails)));
+        Assert.Contains(actionModel.Filters, f => ProducesResponseTypeAttribute(f, 503, typeof(ProblemDetails)));
+    }
 }
diff --git a/tests/Ardalis.Result.AspNetCore.UnitTests/ResultStatusMapAddDefaultMap.cs b/tests/Ardalis.Result.AspNetCore.UnitTests/ResultStatusMapAddDefaultMap.cs
@@ -0,0 +1,37 @@
+using Microsoft.AspNetCore.Mvc;
+using Xunit;
+
+namespace Ardalis.Result.AspNetCore.UnitTests;
+
+public class ResultStatusMapAddDefaultMap
+{
+    [Fact]
+    public void ReturnsBadRequestGivenInvalid()
+    {
+        var controller = new TestController();
+
+        var result = Result<int>.Invalid(new ValidationError("test"));
+
+        // TODO: require identifier
+        Assert.True(true);
+    }
+
+    public class TestController : ControllerBase
+    {
+        public Result Index()
+        {
+            throw new NotImplementedException();
+        }
+
+        public Result<string> ResultString()
+        {
+            throw new NotImplementedException();
+        }
+
+        public Result<IEnumerable<string>> ResultEnumerableString()
+        {
+            throw new NotImplementedException();
+        }
+    }
+
+}
diff --git a/tests/Ardalis.Result.UnitTests/SystemTextJsonSerializer.cs b/tests/Ardalis.Result.UnitTests/SystemTextJsonSerializer.cs
@@ -9,7 +9,7 @@ public class SystemTextJsonSerializer
         public void ShouldSerializeResultOfValueType()
         {
             var result = Result.Success(5);
-            string expected = "{\"Value\":5,\"Status\":0,\"IsSuccess\":true,\"SuccessMessage\":\"\",\"CorrelationId\":\"\",\"Errors\":[],\"ValidationErrors\":[]}";
+            string expected = "{\"Value\":5,\"Status\":0,\"IsSuccess\":true,\"SuccessMessage\":\"\",\"CorrelationId\":\"\",\"Location\":\"\",\"Errors\":[],\"ValidationErrors\":[]}";
 
             var json = JsonSerializer.Serialize(result);
 
@@ -20,7 +20,7 @@ public void ShouldSerializeResultOfValueType()
         public void ShouldSerializeResultOfReferenceType()
         {
             var result = Result.Success(new Foo { Bar = "Result!" });
-            string expected = "{\"Value\":{\"Bar\":\"Result!\"},\"Status\":0,\"IsSuccess\":true,\"SuccessMessage\":\"\",\"CorrelationId\":\"\",\"Errors\":[],\"ValidationErrors\":[]}";
+            string expected = "{\"Value\":{\"Bar\":\"Result!\"},\"Status\":0,\"IsSuccess\":true,\"SuccessMessage\":\"\",\"CorrelationId\":\"\",\"Location\":\"\",\"Errors\":[],\"ValidationErrors\":[]}";
 
             var json = JsonSerializer.Serialize(result);
 

diff --git a/tests/Directory.Build.props b/tests/Directory.Build.props
@@ -1,7 +1,7 @@
 <Project>
   <PropertyGroup>
     <NetCoreFrameworks>net6.0;net7.0;net8.0</NetCoreFrameworks>
-    <TargetFrameworks>net4.7.1;$(NetCoreFrameworks)</TargetFrameworks>
+    <TargetFrameworks>net48;$(NetCoreFrameworks)</TargetFrameworks>
     <IsPackable>false</IsPackable>
     <LangVersion>latest</LangVersion>
   </PropertyGroup>