From bed3542a9ba7b1504882e7d34e2b3d6af63afdb5 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Fri, 21 Jun 2019 18:15:59 -0700 Subject: [PATCH 1/3] Add JsonExtensionData to ProblemDetails Fixes https://github.com/aspnet/AspNetCore/issues/6202 --- .../SystemTextJsonOutputFormatter.cs | 7 +- src/Mvc/Mvc.Core/src/ProblemDetails.cs | 2 + .../Mvc.FunctionalTests/ApiBehaviorTest.cs | 192 +++++++++++++----- .../SystemTextJsonOutputFormatterTest.cs | 25 ++- .../BasicWebSite/StartupWithSystemTextJson.cs | 2 + 5 files changed, 174 insertions(+), 54 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs index fc5eba60ec..964b2f565f 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonOutputFormatter.cs @@ -59,7 +59,12 @@ namespace Microsoft.AspNetCore.Mvc.Formatters 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 index 19691cae5c..e5e1ad31c3 100644 --- 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 @@ namespace Microsoft.AspNetCore.Mvc /// The round-tripping behavior for 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. /// + [JsonExtensionData] public IDictionary Extensions { get; } = new Dictionary(StringComparer.Ordinal); } } diff --git a/src/Mvc/test/Mvc.FunctionalTests/ApiBehaviorTest.cs b/src/Mvc/test/Mvc.FunctionalTests/ApiBehaviorTest.cs index 1005a664f8..11ec57891f 100644 --- 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 @@ using Xunit; namespace Microsoft.AspNetCore.Mvc.FunctionalTests { - public class ApiBehaviorTest : IClassFixture> + public abstract class ApiBehaviorTestBase : IClassFixture> where TStartup : class { - public ApiBehaviorTest(MvcTestFixture fixture) + protected ApiBehaviorTestBase(MvcTestFixture 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(); + builder.UseStartup(); 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 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests 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 - { - {"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>(content); - Assert.Equal(expected, actual); - } - [Fact] public Task ActionsWithApiBehavior_InferFromBodyParameters() => ActionsWithApiBehaviorInferFromBodyParameters("ActionWithInferredFromBodyParameter"); @@ -171,7 +139,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests var response = await Client.PostAsJsonAsync($"/contact/{action}", input); // Assert - Assert.Equal(HttpStatusCode.OK, response.StatusCode); + await response.AssertStatusCodeAsync(HttpStatusCode.OK); var result = JsonConvert.DeserializeObject(await response.Content.ReadAsStringAsync()); Assert.Equal(input.ContactId, result.ContactId); Assert.Equal(input.Name, result.Name); @@ -188,7 +156,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests 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(await response.Content.ReadAsStringAsync()); Assert.Equal(id, result.ContactId); Assert.Equal(name, result.Name); @@ -208,7 +176,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests var response = await Client.GetAsync(url); // Assert - Assert.Equal(HttpStatusCode.OK, response.StatusCode); + await response.AssertStatusCodeAsync(HttpStatusCode.OK); var result = await response.Content.ReadAsAsync(); Assert.Equal(id, result.ContactId); @@ -229,7 +197,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests var response = await Client.GetAsync(url); // Assert - Assert.Equal(HttpStatusCode.OK, response.StatusCode); + await response.AssertStatusCodeAsync(HttpStatusCode.OK); var result = await response.Content.ReadAsAsync(); Assert.Equal(id, result.ContactId); @@ -247,7 +215,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests 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 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests 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 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests } [Fact] - public async Task SerializingProblemDetails_IgnoresNullValuedProperties() + public virtual async Task SerializingProblemDetails_IgnoresNullValuedProperties() { // Arrange var expected = new[] { "status", "title", "traceId", "type" }; @@ -314,7 +282,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests } [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 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests } [Fact] - public async Task SerializingValidationProblemDetails_WithExtensionData() + public virtual async Task SerializingValidationProblemDetails_WithExtensionData() { // Act var response = await Client.GetAsync("/contact/ActionReturningValidationProblemDetails"); @@ -364,4 +332,132 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests }); } } + + public class ApiBehaviorTest : ApiBehaviorTestBase + { + public ApiBehaviorTest(MvcTestFixture 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( + 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(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 + { + public ApiBehaviorTestNewtonsoftJson(MvcTestFixture fixture) + : base(fixture) + { + var factory = fixture.WithWebHostBuilder(ConfigureWebHostBuilder); + CustomInvalidModelStateClient = factory.CreateDefaultClient(); + } + + private static void ConfigureWebHostBuilder(IWebHostBuilder builder) => + builder.UseStartup(); + + 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 + { + {"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>(content); + Assert.Equal(expected, actual); + } + } } diff --git a/src/Mvc/test/Mvc.FunctionalTests/SystemTextJsonOutputFormatterTest.cs b/src/Mvc/test/Mvc.FunctionalTests/SystemTextJsonOutputFormatterTest.cs index 15af231bea..eb1b9493c5 100644 --- 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 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests { } - [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 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests 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(); - [Fact(Skip = "https://github.com/dotnet/corefx/issues/36166")] + // Act + var response = await Client.GetAsync($"/JsonOutputFormatter/{nameof(JsonOutputFormatterController.ProblemDetailsResult)}"); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.NotFound); + + var obj = JObject.Parse(await response.Content.ReadAsStringAsync()); + Assert.Equal("https://tools.ietf.org/html/rfc7231#section-6.5.4", obj.Value("type")); + Assert.Equal("Not Found", obj.Value("title")); + Assert.Equal("404", obj.Value("status")); + } + + [Fact] public override Task Formatting_PolymorphicModel() => base.Formatting_PolymorphicModel(); } } \ No newline at end of file diff --git a/src/Mvc/test/WebSites/BasicWebSite/StartupWithSystemTextJson.cs b/src/Mvc/test/WebSites/BasicWebSite/StartupWithSystemTextJson.cs index 5d5f9d8e91..f5e0c308e5 100644 --- a/src/Mvc/test/WebSites/BasicWebSite/StartupWithSystemTextJson.cs +++ b/src/Mvc/test/WebSites/BasicWebSite/StartupWithSystemTextJson.cs @@ -14,6 +14,8 @@ namespace BasicWebSite services .AddMvc() .SetCompatibilityVersion(CompatibilityVersion.Latest); + + services.AddSingleton(); } public void Configure(IApplicationBuilder app) From 6b36c377f6c1214beaf4cda0b5d6f7e504b036c8 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Mon, 24 Jun 2019 14:27:16 -0700 Subject: [PATCH 2/3] Update ref assemblies --- .../Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs b/src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs index d2eb14ebd9..71a66855d2 100644 --- a/src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs +++ b/src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs @@ -960,6 +960,7 @@ namespace Microsoft.AspNetCore.Mvc { public ProblemDetails() { } public string Detail { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } + [System.Text.Json.Serialization.JsonExtensionDataAttribute] public System.Collections.Generic.IDictionary Extensions { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } public string Instance { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public int? Status { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } From 6c5374db0d9c48a4e389dbf97a96384e5e4e5be2 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Mon, 24 Jun 2019 16:14:46 -0700 Subject: [PATCH 3/3] Changes per PR --- .../Mvc.FunctionalTests/ApiBehaviorTest.cs | 53 ++----------------- .../SystemTextJsonOutputFormatterTest.cs | 18 +------ 2 files changed, 5 insertions(+), 66 deletions(-) diff --git a/src/Mvc/test/Mvc.FunctionalTests/ApiBehaviorTest.cs b/src/Mvc/test/Mvc.FunctionalTests/ApiBehaviorTest.cs index 11ec57891f..0de827952f 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/ApiBehaviorTest.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/ApiBehaviorTest.cs @@ -340,57 +340,10 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests { } - [Fact] - public override async Task ActionsReturnBadRequest_WhenModelStateIsInvalid() + [Fact(Skip = "https://github.com/aspnet/AspNetCore/pull/11460")] + public override 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( - 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(kvp.Value); - Assert.Equal("traceId", Assert.Single(jObject.Properties()).Name); - }); + return base.ActionsReturnBadRequest_WhenModelStateIsInvalid(); } [Fact(Skip = "https://github.com/dotnet/corefx/issues/38769")] diff --git a/src/Mvc/test/Mvc.FunctionalTests/SystemTextJsonOutputFormatterTest.cs b/src/Mvc/test/Mvc.FunctionalTests/SystemTextJsonOutputFormatterTest.cs index eb1b9493c5..14ab2b4845 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/SystemTextJsonOutputFormatterTest.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/SystemTextJsonOutputFormatterTest.cs @@ -33,22 +33,8 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests [Fact] public override Task Formatting_DictionaryType() => base.Formatting_DictionaryType(); - [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); - - var obj = JObject.Parse(await response.Content.ReadAsStringAsync()); - Assert.Equal("https://tools.ietf.org/html/rfc7231#section-6.5.4", obj.Value("type")); - Assert.Equal("Not Found", obj.Value("title")); - Assert.Equal("404", obj.Value("status")); - } + [Fact(Skip = "https://github.com/aspnet/AspNetCore/issues/11522")] + public override Task Formatting_ProblemDetails() => base.Formatting_ProblemDetails(); [Fact] public override Task Formatting_PolymorphicModel() => base.Formatting_PolymorphicModel();