Add JsonExtensionData to ProblemDetails

pranavkm · pranavkm · commit bed3542a9ba7 · 2019-06-24T16:01:02.000-07:00
Fixes #6202
diff --git a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs
@@ -59,7 +59,12 @@ public sealed override async Task WriteResponseBodyAsync(OutputFormatterWriteCon
             var writeStream = GetWriteStream(httpContext, selectedEncoding);
             try
             {
-                await JsonSerializer.WriteAsync(writeStream, context.Object, context.ObjectType, SerializerOptions);
+                // context.ObjectType reflects the declared model type when specified.
+                // For polymorphic scenarios where the user declares a return type, but returns a derived type,
+                // we want to serialize all the properties on the derived type. This keeps parity with
+                // the behavior you get when the user does not declare the return type and with Json.Net at least at the top level.
+                var objectType = context.Object?.GetType() ?? context.ObjectType;
+                await JsonSerializer.WriteAsync(writeStream, context.Object, objectType, SerializerOptions);
                 await writeStream.FlushAsync();
             }
             finally
diff --git a/src/Mvc/Mvc.Core/src/ProblemDetails.cs b/src/Mvc/Mvc.Core/src/ProblemDetails.cs
@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Text.Json.Serialization;
 
 namespace Microsoft.AspNetCore.Mvc
 {
@@ -52,6 +53,7 @@ public class ProblemDetails
         /// The round-tripping behavior for <see cref="Extensions"/> is determined by the implementation of the Input \ Output formatters.
         /// In particular, complex types or collection types may not round-trip to the original type when using the built-in JSON or XML formatters.
         /// </remarks>
+        [JsonExtensionData]
         public IDictionary<string, object> Extensions { get; } = new Dictionary<string, object>(StringComparer.Ordinal);
     }
 }
diff --git a/src/Mvc/test/Mvc.FunctionalTests/ApiBehaviorTest.cs b/src/Mvc/test/Mvc.FunctionalTests/ApiBehaviorTest.cs
@@ -2,7 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System.Collections.Generic;
-using System.Diagnostics;
 using System.Linq;
 using System.Net;
 using System.Net.Http;
@@ -17,24 +16,21 @@
 
 namespace Microsoft.AspNetCore.Mvc.FunctionalTests
 {
-    public class ApiBehaviorTest : IClassFixture<MvcTestFixture<BasicWebSite.StartupWithoutEndpointRouting>>
+    public abstract class ApiBehaviorTestBase<TStartup> : IClassFixture<MvcTestFixture<TStartup>> where TStartup : class
     {
-        public ApiBehaviorTest(MvcTestFixture<BasicWebSite.StartupWithoutEndpointRouting> fixture)
+        protected ApiBehaviorTestBase(MvcTestFixture<TStartup> fixture)
         {
-            Client = fixture.CreateDefaultClient();
-
-            var factory = fixture.WithWebHostBuilder(ConfigureWebHostBuilder);
-            CustomInvalidModelStateClient = factory.CreateDefaultClient();
+            var factory = fixture.Factories.FirstOrDefault() ?? fixture.WithWebHostBuilder(ConfigureWebHostBuilder);
+            Client = factory.CreateDefaultClient();
         }
 
         private static void ConfigureWebHostBuilder(IWebHostBuilder builder) =>
-            builder.UseStartup<BasicWebSite.StartupWithCustomInvalidModelStateFactory>();
+            builder.UseStartup<TStartup>();
 
         public HttpClient Client { get; }
-        public HttpClient CustomInvalidModelStateClient { get; }
 
         [Fact]
-        public async Task ActionsReturnBadRequest_WhenModelStateIsInvalid()
+        public virtual async Task ActionsReturnBadRequest_WhenModelStateIsInvalid()
         {
             // Arrange
             using (new ActivityReplacer())
@@ -122,34 +118,6 @@ public async Task ActionsReturnUnsupportedMediaType_WhenEncodingIsUnsupported()
             Assert.Equal("Unsupported Media Type", problemDetails.Title);
         }
 
-        [Fact]
-        public async Task ActionsReturnBadRequest_UsesProblemDescriptionProviderAndApiConventionsToConfigureErrorResponse()
-        {
-            // Arrange
-            var contactModel = new Contact
-            {
-                Name = "Abc",
-                City = "Redmond",
-                State = "WA",
-                Zip = "Invalid",
-            };
-            var expected = new Dictionary<string, string[]>
-            {
-                {"Name", new[] {"The field Name must be a string with a minimum length of 5 and a maximum length of 30."}},
-                {"Zip", new[] { @"The field Zip must match the regular expression '\d{5}'."}}
-            };
-
-            // Act
-            var response = await CustomInvalidModelStateClient.PostAsJsonAsync("/contact/PostWithVnd", contactModel);
-
-            // Assert
-            await response.AssertStatusCodeAsync(HttpStatusCode.BadRequest);
-            Assert.Equal("application/vnd.error+json", response.Content.Headers.ContentType.MediaType);
-            var content = await response.Content.ReadAsStringAsync();
-            var actual = JsonConvert.DeserializeObject<Dictionary<string, string[]>>(content);
-            Assert.Equal(expected, actual);
-        }
-
         [Fact]
         public Task ActionsWithApiBehavior_InferFromBodyParameters()
             => ActionsWithApiBehaviorInferFromBodyParameters("ActionWithInferredFromBodyParameter");
@@ -171,7 +139,7 @@ private async Task ActionsWithApiBehaviorInferFromBodyParameters(string action)
             var response = await Client.PostAsJsonAsync($"/contact/{action}", input);
 
             // Assert
-            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+            await response.AssertStatusCodeAsync(HttpStatusCode.OK);
             var result = JsonConvert.DeserializeObject<Contact>(await response.Content.ReadAsStringAsync());
             Assert.Equal(input.ContactId, result.ContactId);
             Assert.Equal(input.Name, result.Name);
@@ -188,7 +156,7 @@ public async Task ActionsWithApiBehavior_InferQueryAndRouteParameters()
             var response = await Client.PostAsync(url, new StringContent(string.Empty));
 
             // Assert
-            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+            await response.AssertStatusCodeAsync(HttpStatusCode.OK);
             var result = JsonConvert.DeserializeObject<Contact>(await response.Content.ReadAsStringAsync());
             Assert.Equal(id, result.ContactId);
             Assert.Equal(name, result.Name);
@@ -208,7 +176,7 @@ public async Task ActionsWithApiBehavior_InferEmptyPrefixForComplexValueProvider
             var response = await Client.GetAsync(url);
 
             // Assert
-            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+            await response.AssertStatusCodeAsync(HttpStatusCode.OK);
 
             var result = await response.Content.ReadAsAsync<Contact>();
             Assert.Equal(id, result.ContactId);
@@ -229,7 +197,7 @@ public async Task ActionsWithApiBehavior_InferEmptyPrefixForComplexValueProvider
             var response = await Client.GetAsync(url);
 
             // Assert
-            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+            await response.AssertStatusCodeAsync(HttpStatusCode.OK);
 
             var result = await response.Content.ReadAsAsync<Contact>();
             Assert.Equal(id, result.ContactId);
@@ -247,7 +215,7 @@ public async Task ActionsWithApiBehavior_InferModelBinderType()
             var response = await Client.GetAsync("/contact/ActionWithInferredModelBinderType?foo=Hello!");
 
             // Assert
-            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+            await response.AssertStatusCodeAsync(HttpStatusCode.OK);
             var result = await response.Content.ReadAsStringAsync();
             Assert.Equal(expected, result);
         }
@@ -262,13 +230,13 @@ public async Task ActionsWithApiBehavior_InferModelBinderTypeWithExplicitModelNa
             var response = await Client.GetAsync("/contact/ActionWithInferredModelBinderTypeWithExplicitModelName?bar=Hello!");
 
             // Assert
-            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+            await response.AssertStatusCodeAsync(HttpStatusCode.OK);
             var result = await response.Content.ReadAsStringAsync();
             Assert.Equal(expected, result);
         }
 
         [Fact]
-        public async Task ClientErrorResultFilterExecutesForStatusCodeResults()
+        public virtual async Task ClientErrorResultFilterExecutesForStatusCodeResults()
         {
             using (new ActivityReplacer())
             {
@@ -296,7 +264,7 @@ public async Task ClientErrorResultFilterExecutesForStatusCodeResults()
         }
 
         [Fact]
-        public async Task SerializingProblemDetails_IgnoresNullValuedProperties()
+        public virtual async Task SerializingProblemDetails_IgnoresNullValuedProperties()
         {
             // Arrange
             var expected = new[] { "status", "title", "traceId", "type" };
@@ -314,7 +282,7 @@ public async Task SerializingProblemDetails_IgnoresNullValuedProperties()
         }
 
         [Fact]
-        public async Task SerializingProblemDetails_WithAllValuesSpecified()
+        public virtual async Task SerializingProblemDetails_WithAllValuesSpecified()
         {
             // Arrange
             var expected = new[] { "detail", "instance", "status", "title", "tracking-id", "type" };
@@ -330,7 +298,7 @@ public async Task SerializingProblemDetails_WithAllValuesSpecified()
         }
 
         [Fact]
-        public async Task SerializingValidationProblemDetails_WithExtensionData()
+        public virtual async Task SerializingValidationProblemDetails_WithExtensionData()
         {
             // Act
             var response = await Client.GetAsync("/contact/ActionReturningValidationProblemDetails");
@@ -364,4 +332,132 @@ public async Task SerializingValidationProblemDetails_WithExtensionData()
                 });
         }
     }
+
+    public class ApiBehaviorTest : ApiBehaviorTestBase<BasicWebSite.StartupWithSystemTextJson>
+    {
+        public ApiBehaviorTest(MvcTestFixture<BasicWebSite.StartupWithSystemTextJson> fixture)
+            : base(fixture)
+        {
+        }
+
+        [Fact]
+        public override async Task ActionsReturnBadRequest_WhenModelStateIsInvalid()
+        {
+            // Arrange
+            using var _ = new ActivityReplacer();
+
+            var contactModel = new Contact
+            {
+                Name = "Abc",
+                City = "Redmond",
+                State = "WA",
+                Zip = "Invalid",
+            };
+            var contactString = JsonConvert.SerializeObject(contactModel);
+
+            // Act
+            var response = await Client.PostAsJsonAsync("/contact", contactModel);
+
+            // Assert
+            await response.AssertStatusCodeAsync(HttpStatusCode.BadRequest);
+            Assert.Equal("application/problem+json", response.Content.Headers.ContentType.MediaType);
+            var problemDetails = JsonConvert.DeserializeObject<ValidationProblemDetails>(
+                await response.Content.ReadAsStringAsync(),
+                new JsonSerializerSettings
+                {
+                    Converters = { new ValidationProblemDetailsConverter() }
+                });
+            Assert.Collection(
+                problemDetails.Errors.OrderBy(kvp => kvp.Key),
+                kvp =>
+                {
+                    Assert.Equal("Name", kvp.Key);
+                    var error = Assert.Single(kvp.Value);
+                    Assert.Equal("The field Name must be a string with a minimum length of 5 and a maximum length of 30.", error);
+                },
+                kvp =>
+                {
+                    Assert.Equal("Zip", kvp.Key);
+                    var error = Assert.Single(kvp.Value);
+                    Assert.Equal("The field Zip must match the regular expression '\\d{5}'.", error);
+                }
+            );
+
+            Assert.Collection(
+                problemDetails.Extensions,
+                 kvp =>
+                 {
+                     Assert.Equal("extensions", kvp.Key);
+                     var jObject = Assert.IsType<JObject>(kvp.Value);
+                     Assert.Equal("traceId", Assert.Single(jObject.Properties()).Name);
+                 });
+        }
+
+        [Fact(Skip = "https://github.com/dotnet/corefx/issues/38769")]
+        public override Task ClientErrorResultFilterExecutesForStatusCodeResults()
+        {
+            return base.ClientErrorResultFilterExecutesForStatusCodeResults();
+        }
+
+        [Fact(Skip = "https://github.com/dotnet/corefx/issues/38769")]
+        public override Task SerializingProblemDetails_IgnoresNullValuedProperties()
+        {
+            return base.SerializingProblemDetails_IgnoresNullValuedProperties();
+        }
+
+        [Fact(Skip = "https://github.com/dotnet/corefx/issues/38769")]
+        public override Task SerializingProblemDetails_WithAllValuesSpecified()
+        {
+            return base.SerializingProblemDetails_WithAllValuesSpecified();
+        }
+
+        [Fact(Skip = "https://github.com/dotnet/corefx/issues/38769")]
+        public override Task SerializingValidationProblemDetails_WithExtensionData()
+        {
+            return base.SerializingValidationProblemDetails_WithExtensionData();
+        }
+    }
+
+    public class ApiBehaviorTestNewtonsoftJson : ApiBehaviorTestBase<BasicWebSite.StartupWithoutEndpointRouting>
+    {
+        public ApiBehaviorTestNewtonsoftJson(MvcTestFixture<BasicWebSite.StartupWithoutEndpointRouting> fixture)
+            : base(fixture)
+        {
+            var factory = fixture.WithWebHostBuilder(ConfigureWebHostBuilder);
+            CustomInvalidModelStateClient = factory.CreateDefaultClient();
+        }
+
+        private static void ConfigureWebHostBuilder(IWebHostBuilder builder) =>
+            builder.UseStartup<BasicWebSite.StartupWithCustomInvalidModelStateFactory>();
+
+        public HttpClient CustomInvalidModelStateClient { get; }
+
+        [Fact]
+        public async Task ActionsReturnBadRequest_UsesProblemDescriptionProviderAndApiConventionsToConfigureErrorResponse()
+        {
+            // Arrange
+            var contactModel = new Contact
+            {
+                Name = "Abc",
+                City = "Redmond",
+                State = "WA",
+                Zip = "Invalid",
+            };
+            var expected = new Dictionary<string, string[]>
+            {
+                {"Name", new[] {"The field Name must be a string with a minimum length of 5 and a maximum length of 30."}},
+                {"Zip", new[] { @"The field Zip must match the regular expression '\d{5}'."}}
+            };
+
+            // Act
+            var response = await CustomInvalidModelStateClient.PostAsJsonAsync("/contact/PostWithVnd", contactModel);
+
+            // Assert
+            await response.AssertStatusCodeAsync(HttpStatusCode.BadRequest);
+            Assert.Equal("application/vnd.error+json", response.Content.Headers.ContentType.MediaType);
+            var content = await response.Content.ReadAsStringAsync();
+            var actual = JsonConvert.DeserializeObject<Dictionary<string, string[]>>(content);
+            Assert.Equal(expected, actual);
+        }
+    }
 }
diff --git a/src/Mvc/test/Mvc.FunctionalTests/SystemTextJsonOutputFormatterTest.cs b/src/Mvc/test/Mvc.FunctionalTests/SystemTextJsonOutputFormatterTest.cs
@@ -4,6 +4,7 @@
 using System.Net;
 using System.Threading.Tasks;
 using FormatterWebSite.Controllers;
+using Newtonsoft.Json.Linq;
 using Xunit;
 
 namespace Microsoft.AspNetCore.Mvc.FunctionalTests
@@ -15,7 +16,7 @@ public SystemTextJsonOutputFormatterTest(MvcTestFixture<FormatterWebSite.Startup
         {
         }
 
-        [Fact(Skip = "Dictionary serialization does not correctly work.")]
+        [Fact(Skip = "https://github.com/aspnet/AspNetCore/issues/11459")]
         public override Task SerializableErrorIsReturnedInExpectedFormat() => base.SerializableErrorIsReturnedInExpectedFormat();
 
         [Fact]
@@ -29,13 +30,27 @@ public override async Task Formatting_StringValueWithUnicodeContent()
             Assert.Equal("\"Hello Mr. \\ud83e\\udd8a\"", await response.Content.ReadAsStringAsync());
         }
 
-        [Fact(Skip = "Dictionary serialization does not correctly work.")]
+        [Fact]
         public override Task Formatting_DictionaryType() => base.Formatting_DictionaryType();
 
-        [Fact(Skip = "Dictionary serialization does not correctly work.")]
-        public override Task Formatting_ProblemDetails() => base.Formatting_ProblemDetails();
+        [Fact]
+        public override async Task Formatting_ProblemDetails()
+        {
+            using var _ = new ActivityReplacer();
+
+            // Act
+            var response = await Client.GetAsync($"/JsonOutputFormatter/{nameof(JsonOutputFormatterController.ProblemDetailsResult)}");
+
+            // Assert
+            await response.AssertStatusCodeAsync(HttpStatusCode.NotFound);
 
-        [Fact(Skip = "https://github.com/dotnet/corefx/issues/36166")]
+            var obj = JObject.Parse(await response.Content.ReadAsStringAsync());
+            Assert.Equal("https://tools.ietf.org/html/rfc7231#section-6.5.4", obj.Value<string>("type"));
+            Assert.Equal("Not Found", obj.Value<string>("title"));
+            Assert.Equal("404", obj.Value<string>("status"));
+        }
+
+        [Fact]
         public override Task Formatting_PolymorphicModel() => base.Formatting_PolymorphicModel();
     }
 }
diff --git a/src/Mvc/test/WebSites/BasicWebSite/StartupWithSystemTextJson.cs b/src/Mvc/test/WebSites/BasicWebSite/StartupWithSystemTextJson.cs
@@ -14,6 +14,8 @@ public void ConfigureServices(IServiceCollection services)
             services
                 .AddMvc()
                 .SetCompatibilityVersion(CompatibilityVersion.Latest);
+
+            services.AddSingleton<ContactsRepository>();
         }
 
         public void Configure(IApplicationBuilder app)

Original file line number	Diff line number	Diff line change
`@@ -59,7 +59,12 @@ public sealed override async Task WriteResponseBodyAsync(OutputFormatterWriteCon`
`59`	`59`	`var writeStream = GetWriteStream(httpContext, selectedEncoding);`
`60`	`60`	`try`
`61`	`61`	`{`
`62`		`- await JsonSerializer.WriteAsync(writeStream, context.Object, context.ObjectType, SerializerOptions);`
	`62`	`+ // context.ObjectType reflects the declared model type when specified.`
	`63`	`+ // For polymorphic scenarios where the user declares a return type, but returns a derived type,`
	`64`	`+ // we want to serialize all the properties on the derived type. This keeps parity with`
	`65`	`+ // the behavior you get when the user does not declare the return type and with Json.Net at least at the top level.`
	`66`	`+ var objectType = context.Object?.GetType() ?? context.ObjectType;`
	`67`	`+ await JsonSerializer.WriteAsync(writeStream, context.Object, objectType, SerializerOptions);`
`63`	`68`	`await writeStream.FlushAsync();`
`64`	`69`	`}`
`65`	`70`	`finally`
Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,7 @@`
`3`	`3`
`4`	`4`	`using System;`
`5`	`5`	`using System.Collections.Generic;`
	`6`	`+using System.Text.Json.Serialization;`
`6`	`7`
`7`	`8`	`namespace Microsoft.AspNetCore.Mvc`
`8`	`9`	`{`
`@@ -52,6 +53,7 @@ public class ProblemDetails`
`52`	`53`	`/// The round-tripping behavior for <see cref="Extensions"/> is determined by the implementation of the Input \ Output formatters.`
`53`	`54`	`/// In particular, complex types or collection types may not round-trip to the original type when using the built-in JSON or XML formatters.`
`54`	`55`	`/// </remarks>`
	`56`	`+ [JsonExtensionData]`
`55`	`57`	`public IDictionary<string, object> Extensions { get; } = new Dictionary<string, object>(StringComparer.Ordinal);`
`56`	`58`	`}`
`57`	`59`	`}`
Original file line number	Diff line number	Diff line change
`@@ -14,6 +14,8 @@ public void ConfigureServices(IServiceCollection services)`
`14`	`14`	`services`
`15`	`15`	`.AddMvc()`
`16`	`16`	`.SetCompatibilityVersion(CompatibilityVersion.Latest);`
	`17`	`+`
	`18`	`+ services.AddSingleton<ContactsRepository>();`
`17`	`19`	`}`
`18`	`20`
`19`	`21`	`public void Configure(IApplicationBuilder app)`