From 5f7ee466a93f2cfc43aed170af4b35bfc9206ea8 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Wed, 29 Oct 2014 16:20:24 -0700 Subject: [PATCH 1/5] Fix #1370 - Always use the provided formatter in JsonResult The change here is to always use the provided formatter, instead of using it as a fallback. This is much less surprising for users. There are some other subtle changes here and cleanup of the tests, as well as documentation additions. The primary change is that we still want to run 'select' on a formatter even if it's the only one. This allows us to choose a content type based on the accept header. In the case of a user-provided formatter, we'll try to honor the best possible combination of Accept and specified ContentTypes (specified ContentTypes win if there's a conflict). If nothing works, we'll still run the user-provided formatter and let it decide what to do. In the case of the default (formatters from options) we do conneg, and if there's a conflict, fall back to a global (from services) JsonOutputFormatter - we let it decide what to do. This should leave us with a defined and tested behavior for all cases. --- .../ActionResults/JsonResult.cs | 142 +++++++------ .../JsonResultTest.cs | 188 +++++++++++------- .../JsonResultTest.cs | 140 +++++++++++++ .../Controllers/JsonResultController.cs | 35 ++++ 4 files changed, 371 insertions(+), 134 deletions(-) create mode 100644 test/Microsoft.AspNet.Mvc.FunctionalTests/JsonResultTest.cs create mode 100644 test/WebSites/BasicWebSite/Controllers/JsonResultController.cs diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs index 4ed6f1e06d..686df40c35 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs @@ -2,111 +2,121 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Collections.Generic; -using System.Linq; using System.Threading.Tasks; using Microsoft.AspNet.Mvc.HeaderValueAbstractions; using Microsoft.Framework.DependencyInjection; namespace Microsoft.AspNet.Mvc { + /// + /// An action result which formats the given object as JSON. + /// public class JsonResult : ActionResult { - private static readonly IList _defaultSupportedContentTypes = - new List() - { - MediaTypeHeaderValue.Parse("application/json"), - MediaTypeHeaderValue.Parse("text/json"), - }; - private IOutputFormatter _defaultFormatter; - - private ObjectResult _objectResult; - - public JsonResult(object data) : - this(data, null) + private static readonly MediaTypeHeaderValue[] _defaultSupportedContentTypes = new MediaTypeHeaderValue[] { - } + MediaTypeHeaderValue.Parse("application/json"), + MediaTypeHeaderValue.Parse("text/json"), + }; /// - /// Creates an instance of class. + /// Creates a new with the given . /// - /// - /// If no matching formatter is found, - /// the response is written to using defaultFormatter. - /// - /// The default formatter must be able to handle either application/json - /// or text/json. - /// - public JsonResult(object data, IOutputFormatter defaultFormatter) + /// The value to format as JSON. + public JsonResult(object value) + : this(value, null) { - _defaultFormatter = defaultFormatter; - _objectResult = new ObjectResult(data); } - public object Value + /// + /// Creates a new with the given . + /// + /// The value to format as JSON. + /// The formatter to use, or null to choose a formatter dynamically. + public JsonResult(object value, IOutputFormatter formatter) { - get - { - return _objectResult.Value; - } - set - { - _objectResult.Value = value; - } - } + Value = value; + Formatter = formatter; - public IList ContentTypes - { - get - { - return _objectResult.ContentTypes; - } - set - { - _objectResult.ContentTypes = value; - } + ContentTypes = new List(); } + /// + /// Gets or sets the list of supported Content-Types. + /// + public IList ContentTypes { get; set; } + + /// + /// Gets or sets the formatter. + /// + public IOutputFormatter Formatter { get; set; } + + /// + /// Gets or sets the value to be formatted. + /// + public object Value { get; set; } + + /// public override async Task ExecuteResultAsync([NotNull] ActionContext context) { + var objectResult = new ObjectResult(Value); + // Set the content type explicitly to application/json and text/json. // if the user has not already set it. if (ContentTypes == null || ContentTypes.Count == 0) { - ContentTypes = _defaultSupportedContentTypes; + objectResult.ContentTypes = _defaultSupportedContentTypes; + } + else + { + objectResult.ContentTypes = ContentTypes; } var formatterContext = new OutputFormatterContext() { - DeclaredType = _objectResult.DeclaredType, ActionContext = context, Object = Value, }; - // Need to call this instead of directly calling _objectResult.ExecuteResultAsync - // as that sets the status to 406 if a formatter is not found. - // this can be cleaned up after https://github.com/aspnet/Mvc/issues/941 gets resolved. - var formatter = SelectFormatter(formatterContext); + var formatter = SelectFormatter(objectResult, formatterContext); await formatter.WriteAsync(formatterContext); } - private IOutputFormatter SelectFormatter(OutputFormatterContext formatterContext) + private IOutputFormatter SelectFormatter(ObjectResult objectResult, OutputFormatterContext formatterContext) { - var defaultFormatters = formatterContext.ActionContext - .HttpContext - .RequestServices - .GetRequiredService() - .OutputFormatters; - - var formatter = _objectResult.SelectFormatter(formatterContext, defaultFormatters); - if (formatter == null) + if (Formatter == null) { - formatter = _defaultFormatter ?? formatterContext.ActionContext - .HttpContext - .RequestServices - .GetRequiredService(); - } + // If no formatter was provided, then run Conneg with the formatters configured in options. + var formatters = formatterContext + .ActionContext + .HttpContext + .RequestServices + .GetRequiredService() + .OutputFormatters; + + var formatter = objectResult.SelectFormatter(formatterContext, formatters); + if (formatter == null) + { + // If the available user-configured formatters can't write this type, then fall back to the + // 'global' one. + formatter = formatterContext + .ActionContext + .HttpContext + .RequestServices + .GetRequiredService(); + + // Run SelectFormatter again to try to choose a content type that this formatter can do. + objectResult.SelectFormatter(formatterContext, new[] { formatter }); + } - return formatter; + return formatter; + } + else + { + // Run SelectFormatter to try to choose a content type that this formatter can do. + objectResult.SelectFormatter(formatterContext, new[] { Formatter }); + return Formatter; + } } } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/JsonResultTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/JsonResultTest.cs index d09c1c12e5..9c07b7df06 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/JsonResultTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/JsonResultTest.cs @@ -12,7 +12,7 @@ using Microsoft.AspNet.Routing; using Moq; using Xunit; - +using Microsoft.AspNet.PipelineCore; namespace Microsoft.AspNet.Mvc { @@ -22,114 +22,166 @@ private static readonly byte[] _abcdUTF8Bytes = new byte[] { 123, 34, 102, 111, 111, 34, 58, 34, 97, 98, 99, 100, 34, 125 }; [Fact] - public async Task ExecuteResult_GeneratesResultsWithoutBOMByDefault() + public async Task ExecuteResultAsync_OptionsFormatter_WithoutBOM() { // Arrange var expected = _abcdUTF8Bytes; - var memoryStream = new MemoryStream(); - var response = new Mock(); - response.SetupGet(r => r.Body) - .Returns(memoryStream); - var context = GetHttpContext(response.Object); - var actionContext = new ActionContext(context, - new RouteData(), - new ActionDescriptor()); + + var optionsFormatters = new List() + { + new XmlDataContractSerializerOutputFormatter(), // This won't be used + new JsonOutputFormatter(), + }; + + var context = GetHttpContext(optionsFormatters); + var actionContext = new ActionContext(context, new RouteData(), new ActionDescriptor()); + var result = new JsonResult(new { foo = "abcd" }); // Act await result.ExecuteResultAsync(actionContext); + var written = GetWrittenBytes(context); // Assert - Assert.Equal(expected, memoryStream.ToArray()); + Assert.Equal(expected, written); + Assert.Equal("application/json;charset=utf-8", context.Response.ContentType); } [Fact] - public async Task ExecuteResult_IfNoMatchFoundUsesPassedInFormatter() + public async Task ExecuteResultAsync_OptionsFormatter_Conneg() + { + // Arrange + var expected = _abcdUTF8Bytes; + + var optionsFormatters = new List() + { + new XmlDataContractSerializerOutputFormatter(), // This won't be used + new JsonOutputFormatter(), + }; + + var context = GetHttpContext(optionsFormatters); + context.Request.Accept = "text/json"; + + var actionContext = new ActionContext(context, new RouteData(), new ActionDescriptor()); + + var result = new JsonResult(new { foo = "abcd" }); + + // Act + await result.ExecuteResultAsync(actionContext); + var written = GetWrittenBytes(context); + + // Assert + Assert.Equal(expected, written); + Assert.Equal("text/json;charset=utf-8", context.Response.ContentType); + } + + [Fact] + public async Task ExecuteResultAsync_UsesPassedInFormatter() { // Arrange var expected = Enumerable.Concat(Encoding.UTF8.GetPreamble(), _abcdUTF8Bytes); - var memoryStream = new MemoryStream(); - var response = new Mock(); - response.SetupGet(r => r.Body) - .Returns(memoryStream); - var context = GetHttpContext(response.Object, registerDefaultFormatter: false); - var actionContext = new ActionContext(context, - new RouteData(), - new ActionDescriptor()); - var testFormatter = new TestJsonFormatter() - { - Encoding = Encoding.UTF8 - }; - - var result = new JsonResult(new { foo = "abcd" }, testFormatter); + + var context = GetHttpContext(); + var actionContext = new ActionContext(context, new RouteData(), new ActionDescriptor()); + + var formatter = new JsonOutputFormatter(); + formatter.SupportedEncodings.Clear(); + + // This is UTF-8 WITH BOM + formatter.SupportedEncodings.Add(Encoding.UTF8); + + var result = new JsonResult(new { foo = "abcd" }, formatter); // Act await result.ExecuteResultAsync(actionContext); + var written = GetWrittenBytes(context); // Assert - Assert.Equal(expected, memoryStream.ToArray()); + Assert.Equal(expected, written); + Assert.Equal("application/json;charset=utf-8", context.Response.ContentType); } - public async Task ExecuteResult_UsesDefaultFormatter_IfNoneIsRegistered_AndNoneIsPassed() + [Fact] + public async Task ExecuteResultAsync_UsesPassedInFormatter_ContentTypeSpecified() { // Arrange var expected = _abcdUTF8Bytes; - var memoryStream = new MemoryStream(); - var response = new Mock(); - response.SetupGet(r => r.Body) - .Returns(memoryStream); - var context = GetHttpContext(response.Object, registerDefaultFormatter: false); - var actionContext = new ActionContext(context, - new RouteData(), - new ActionDescriptor()); - var result = new JsonResult(new { foo = "abcd" }); + + var context = GetHttpContext(); + var actionContext = new ActionContext(context, new RouteData(), new ActionDescriptor()); + + var formatter = new JsonOutputFormatter(); + formatter.SupportedMediaTypes.Add(MediaTypeHeaderValue.Parse("application/hal+json")); + + var result = new JsonResult(new { foo = "abcd" }, formatter); + result.ContentTypes.Add(MediaTypeHeaderValue.Parse("application/hal+json")); // Act await result.ExecuteResultAsync(actionContext); + var written = GetWrittenBytes(context); // Assert - Assert.Equal(expected, memoryStream.ToArray()); + Assert.Equal(expected, written); + Assert.Equal("application/hal+json;charset=utf-8", context.Response.ContentType); } - private HttpContext GetHttpContext(HttpResponse response, bool registerDefaultFormatter = true) + // If no formatter in options can match the given content-types, then use the one registered + // in services + [Fact] + public async Task ExecuteResultAsync_UsesGlobalFormatter_IfNoFormatterIsConfigured() { - var defaultFormatters = registerDefaultFormatter ? new List() { new JsonOutputFormatter() } : - new List(); - var httpContext = new Mock(); - var mockFormattersProvider = new Mock(); - mockFormattersProvider.SetupGet(o => o.OutputFormatters) - .Returns(defaultFormatters); - httpContext.Setup(o => o.RequestServices.GetService(typeof(IOutputFormattersProvider))) - .Returns(mockFormattersProvider.Object); - httpContext.SetupGet(o => o.Request.Accept) - .Returns(""); - httpContext.SetupGet(c => c.Response).Returns(response); - return httpContext.Object; + // Arrange + var expected = _abcdUTF8Bytes; + + var context = GetHttpContext(enableFallback: true); + var actionContext = new ActionContext(context, new RouteData(), new ActionDescriptor()); + + var result = new JsonResult(new { foo = "abcd" }); + + // Act + await result.ExecuteResultAsync(actionContext); + var written = GetWrittenBytes(context); + + // Assert + Assert.Equal(expected, written); + Assert.Equal("application/json;charset=utf-8", context.Response.ContentType); } - private class TestJsonFormatter : IOutputFormatter + private HttpContext GetHttpContext( + IReadOnlyList optionsFormatters = null, + bool enableFallback = false) { - public Encoding Encoding { get; set; } + var httpContext = new DefaultHttpContext(); + httpContext.Response.Body = new MemoryStream(); - public bool CanWriteResult(OutputFormatterContext context, MediaTypeHeaderValue contentType) - { - return true; - } + var services = new Mock(MockBehavior.Strict); + httpContext.RequestServices = services.Object; - public IReadOnlyList GetSupportedContentTypes(Type declaredType, - Type runtimeType, - MediaTypeHeaderValue contentType) - { - return null; - } + var mockFormattersProvider = new Mock(); + mockFormattersProvider + .SetupGet(o => o.OutputFormatters) + .Returns(optionsFormatters ?? new List()); + + services + .Setup(s => s.GetService(typeof(IOutputFormattersProvider))) + .Returns(mockFormattersProvider.Object); - public async Task WriteAsync(OutputFormatterContext context) + // This is the ultimate fallback, it will be used if none of the formatters from options + // work. + if (enableFallback) { - // Override using the selected encoding. - context.SelectedEncoding = Encoding; - var jsonFormatter = new JsonOutputFormatter(); - await jsonFormatter.WriteResponseBodyAsync(context); + services + .Setup(s => s.GetService(typeof(JsonOutputFormatter))) + .Returns(new JsonOutputFormatter()); } + + return httpContext; + } + + private byte[] GetWrittenBytes(HttpContext context) + { + context.Response.Body.Seek(0, SeekOrigin.Begin); + return Assert.IsType(context.Response.Body).ToArray(); } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/JsonResultTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/JsonResultTest.cs new file mode 100644 index 0000000000..d28c474128 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/JsonResultTest.cs @@ -0,0 +1,140 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Net; +using System.Net.Http; +using System.Threading.Tasks; +using Microsoft.AspNet.Builder; +using Microsoft.AspNet.TestHost; +using Xunit; + +namespace Microsoft.AspNet.Mvc.FunctionalTests +{ + public class JsonResultTest + { + private readonly IServiceProvider _provider = TestHelper.CreateServices(nameof(BasicWebSite)); + private readonly Action _app = new BasicWebSite.Startup().Configure; + + [Theory] + [InlineData("application/json")] + [InlineData("text/json")] + public async Task JsonResult_Conneg(string mediaType) + { + // Arrange + var server = TestServer.Create(_provider, _app); + var client = server.CreateClient(); + + var url = "http://localhost/JsonResult/Plain"; + + var request = new HttpRequestMessage(HttpMethod.Get, url); + request.Headers.TryAddWithoutValidation("Accept", mediaType); + + // Act + var response = await client.SendAsync(request); + var content = await response.Content.ReadAsStringAsync(); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal(mediaType, response.Content.Headers.ContentType.MediaType); + Assert.Equal("{\"Message\":\"hello\"}", content); + } + + // Using an Accept header can't force Json to not be Json. If your accept header doesn't jive with the + // formatters/content-type configured on the result it will be ignored. + [Theory] + [InlineData("application/xml")] + [InlineData("text/xml")] + public async Task JsonResult_Conneg_Fails(string mediaType) + { + // Arrange + var server = TestServer.Create(_provider, _app); + var client = server.CreateClient(); + + var url = "http://localhost/JsonResult/Plain"; + + var request = new HttpRequestMessage(HttpMethod.Get, url); + request.Headers.TryAddWithoutValidation("Accept", mediaType); + + // Act + var response = await client.SendAsync(request); + var content = await response.Content.ReadAsStringAsync(); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal("application/json", response.Content.Headers.ContentType.MediaType); + Assert.Equal("{\"Message\":\"hello\"}", content); + } + + [Theory] + [InlineData("application/json")] + [InlineData("text/json")] + public async Task JsonResult_CustomFormatter_Conneg(string mediaType) + { + // Arrange + var server = TestServer.Create(_provider, _app); + var client = server.CreateClient(); + + var url = "http://localhost/JsonResult/CustomFormatter"; + + var request = new HttpRequestMessage(HttpMethod.Get, url); + request.Headers.TryAddWithoutValidation("Accept", mediaType); + + // Act + var response = await client.SendAsync(request); + var content = await response.Content.ReadAsStringAsync(); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal(mediaType, response.Content.Headers.ContentType.MediaType); + Assert.Equal("{\"message\":\"hello\"}", content); + } + + // Using an Accept header can't force Json to not be Json. If your accept header doesn't jive with the + // formatters/content-type configured on the result it will be ignored. + [Theory] + [InlineData("application/xml")] + [InlineData("text/xml")] + public async Task JsonResult_CustomFormatter_Conneg_Fails(string mediaType) + { + // Arrange + var server = TestServer.Create(_provider, _app); + var client = server.CreateClient(); + + var url = "http://localhost/JsonResult/CustomFormatter"; + + var request = new HttpRequestMessage(HttpMethod.Get, url); + request.Headers.TryAddWithoutValidation("Accept", mediaType); + + // Act + var response = await client.SendAsync(request); + var content = await response.Content.ReadAsStringAsync(); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal("application/json", response.Content.Headers.ContentType.MediaType); + Assert.Equal("{\"message\":\"hello\"}", content); + } + + [Fact] + public async Task JsonResult_CustomContentType() + { + // Arrange + var server = TestServer.Create(_provider, _app); + var client = server.CreateClient(); + + var url = "http://localhost/JsonResult/CustomContentType"; + + var request = new HttpRequestMessage(HttpMethod.Get, url); + + // Act + var response = await client.SendAsync(request); + var content = await response.Content.ReadAsStringAsync(); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal("application/message+json", response.Content.Headers.ContentType.MediaType); + Assert.Equal("{\"Message\":\"hello\"}", content); + } + } +} \ No newline at end of file diff --git a/test/WebSites/BasicWebSite/Controllers/JsonResultController.cs b/test/WebSites/BasicWebSite/Controllers/JsonResultController.cs new file mode 100644 index 0000000000..f684441f8d --- /dev/null +++ b/test/WebSites/BasicWebSite/Controllers/JsonResultController.cs @@ -0,0 +1,35 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNet.Mvc; +using Microsoft.AspNet.Mvc.HeaderValueAbstractions; +using Newtonsoft.Json.Serialization; + +namespace BasicWebSite.Controllers +{ + public class JsonResultController : Controller + { + public JsonResult Plain() + { + return Json(new { Message = "hello" }); + } + + public JsonResult CustomFormatter() + { + var formatter = new JsonOutputFormatter(); + formatter.SerializerSettings.ContractResolver = new CamelCasePropertyNamesContractResolver(); + + return new JsonResult(new { Message = "hello" }, formatter); + } + + public JsonResult CustomContentType() + { + var formatter = new JsonOutputFormatter(); + formatter.SupportedMediaTypes.Add(MediaTypeHeaderValue.Parse("application/message+json")); + + var result = new JsonResult(new { Message = "hello" }, formatter); + result.ContentTypes.Add(MediaTypeHeaderValue.Parse("application/message+json")); + return result; + } + } +} \ No newline at end of file From f874670037f2ac79675f2ee7dea082fb7b603add Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Wed, 29 Oct 2014 18:31:40 -0700 Subject: [PATCH 2/5] cr feedback --- .../ActionResults/JsonResult.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs index 686df40c35..ce39f8d6ee 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs @@ -13,7 +13,10 @@ namespace Microsoft.AspNet.Mvc /// public class JsonResult : ActionResult { - private static readonly MediaTypeHeaderValue[] _defaultSupportedContentTypes = new MediaTypeHeaderValue[] + /// + /// The list of content-types used for formatting when is null or empty. + /// + public static readonly IReadOnlyList DefaultContentTypes = new MediaTypeHeaderValue[] { MediaTypeHeaderValue.Parse("application/json"), MediaTypeHeaderValue.Parse("text/json"), @@ -65,7 +68,10 @@ public override async Task ExecuteResultAsync([NotNull] ActionContext context) // if the user has not already set it. if (ContentTypes == null || ContentTypes.Count == 0) { - objectResult.ContentTypes = _defaultSupportedContentTypes; + foreach (var contentType in DefaultContentTypes) + { + objectResult.ContentTypes.Add(contentType); + } } else { From 27551d3069af6d740048e993efbb8ab99939a237 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Thu, 30 Oct 2014 18:08:47 -0700 Subject: [PATCH 3/5] cr feedback --- .../ActionResults/JsonResult.cs | 1 + .../JsonResultTest.cs | 28 ++++++++++++++++++- .../JsonResultTest.cs | 20 +++++++++++++ .../Controllers/JsonResultController.cs | 5 ++++ 4 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs index ce39f8d6ee..271ae17f80 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs @@ -81,6 +81,7 @@ public override async Task ExecuteResultAsync([NotNull] ActionContext context) var formatterContext = new OutputFormatterContext() { ActionContext = context, + DeclaredType = objectResult.DeclaredType, Object = Value, }; diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/JsonResultTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/JsonResultTest.cs index 9c07b7df06..670ce70b67 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/JsonResultTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/JsonResultTest.cs @@ -9,10 +9,10 @@ using System.Threading.Tasks; using Microsoft.AspNet.Http; using Microsoft.AspNet.Mvc.HeaderValueAbstractions; +using Microsoft.AspNet.PipelineCore; using Microsoft.AspNet.Routing; using Moq; using Xunit; -using Microsoft.AspNet.PipelineCore; namespace Microsoft.AspNet.Mvc { @@ -47,6 +47,32 @@ public async Task ExecuteResultAsync_OptionsFormatter_WithoutBOM() Assert.Equal("application/json;charset=utf-8", context.Response.ContentType); } + [Fact] + public async Task ExecuteResultAsync_Null() + { + // Arrange + var expected = _abcdUTF8Bytes; + + var optionsFormatters = new List() + { + new XmlDataContractSerializerOutputFormatter(), // This won't be used + new JsonOutputFormatter(), + }; + + var context = GetHttpContext(optionsFormatters); + var actionContext = new ActionContext(context, new RouteData(), new ActionDescriptor()); + + var result = new JsonResult(new { foo = "abcd" }); + + // Act + await result.ExecuteResultAsync(actionContext); + var written = GetWrittenBytes(context); + + // Assert + Assert.Equal(expected, written); + Assert.Equal("application/json;charset=utf-8", context.Response.ContentType); + } + [Fact] public async Task ExecuteResultAsync_OptionsFormatter_Conneg() { diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/JsonResultTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/JsonResultTest.cs index d28c474128..a0d61fc4a9 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/JsonResultTest.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/JsonResultTest.cs @@ -66,6 +66,26 @@ public async Task JsonResult_Conneg_Fails(string mediaType) Assert.Equal("{\"Message\":\"hello\"}", content); } + // If the object is null, it will get picked up by the nocontent formatter. + [Fact] + public async Task JsonResult_Null() + { + // Arrange + var server = TestServer.Create(_provider, _app); + var client = server.CreateClient(); + + var url = "http://localhost/JsonResult/Null"; + + var request = new HttpRequestMessage(HttpMethod.Get, url); + + // Act + var response = await client.SendAsync(request); + var content = await response.Content.ReadAsStringAsync(); + + // Assert + Assert.Equal(HttpStatusCode.NoContent, response.StatusCode); + } + [Theory] [InlineData("application/json")] [InlineData("text/json")] diff --git a/test/WebSites/BasicWebSite/Controllers/JsonResultController.cs b/test/WebSites/BasicWebSite/Controllers/JsonResultController.cs index f684441f8d..caf7475446 100644 --- a/test/WebSites/BasicWebSite/Controllers/JsonResultController.cs +++ b/test/WebSites/BasicWebSite/Controllers/JsonResultController.cs @@ -31,5 +31,10 @@ public JsonResult CustomContentType() result.ContentTypes.Add(MediaTypeHeaderValue.Parse("application/message+json")); return result; } + + public JsonResult Null() + { + return Json(null); + } } } \ No newline at end of file From 0e0f3188ddc106346ff36423f275c8f66c8a4f2e Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Fri, 31 Oct 2014 11:18:18 -0700 Subject: [PATCH 4/5] cr feedback --- src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs index 271ae17f80..74e9231f9e 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs @@ -27,7 +27,7 @@ public class JsonResult : ActionResult /// /// The value to format as JSON. public JsonResult(object value) - : this(value, null) + : this(value, formatter: null) { } From 18ff76992494a700e0a75e728720f44e0695e95f Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Fri, 31 Oct 2014 12:46:58 -0700 Subject: [PATCH 5/5] bleh; --- .../ActionResults/JsonResult.cs | 5 +++- .../Formatters/IJsonOutputFormatter.cs | 20 +++++++++++++ .../Formatters/JsonOutputFormatter.cs | 2 +- .../JsonResultTest.cs | 28 +++++++++++++++++-- .../Controllers/JsonResultController.cs | 5 ++++ 5 files changed, 56 insertions(+), 4 deletions(-) create mode 100644 src/Microsoft.AspNet.Mvc.Core/Formatters/IJsonOutputFormatter.cs diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs index 74e9231f9e..2f598152f0 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using Microsoft.AspNet.Mvc.HeaderValueAbstractions; using Microsoft.Framework.DependencyInjection; @@ -99,7 +100,9 @@ private IOutputFormatter SelectFormatter(ObjectResult objectResult, OutputFormat .HttpContext .RequestServices .GetRequiredService() - .OutputFormatters; + .OutputFormatters + .OfType() + .ToArray(); var formatter = objectResult.SelectFormatter(formatterContext, formatters); if (formatter == null) diff --git a/src/Microsoft.AspNet.Mvc.Core/Formatters/IJsonOutputFormatter.cs b/src/Microsoft.AspNet.Mvc.Core/Formatters/IJsonOutputFormatter.cs new file mode 100644 index 0000000000..db86820470 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/Formatters/IJsonOutputFormatter.cs @@ -0,0 +1,20 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNet.Mvc +{ + /// + /// An output formatter that specializes in writing JSON content. + /// + /// + /// The class filter the collection of + /// and use only those which implement + /// . + /// + /// To create a custom formatter that can be used by , derive from + /// or implement . + /// + public interface IJsonOutputFormatter : IOutputFormatter + { + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/Formatters/JsonOutputFormatter.cs b/src/Microsoft.AspNet.Mvc.Core/Formatters/JsonOutputFormatter.cs index 1a017f92d5..55dc17c7e2 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Formatters/JsonOutputFormatter.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Formatters/JsonOutputFormatter.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNet.Mvc { - public class JsonOutputFormatter : OutputFormatter + public class JsonOutputFormatter : OutputFormatter, IJsonOutputFormatter { private JsonSerializerSettings _serializerSettings; diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/JsonResultTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/JsonResultTest.cs index a0d61fc4a9..64e6b527c5 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/JsonResultTest.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/JsonResultTest.cs @@ -66,7 +66,7 @@ public async Task JsonResult_Conneg_Fails(string mediaType) Assert.Equal("{\"Message\":\"hello\"}", content); } - // If the object is null, it will get picked up by the nocontent formatter. + // If the object is null, it will get formatted as JSON. NOT as a 204/NoContent [Fact] public async Task JsonResult_Null() { @@ -83,7 +83,31 @@ public async Task JsonResult_Null() var content = await response.Content.ReadAsStringAsync(); // Assert - Assert.Equal(HttpStatusCode.NoContent, response.StatusCode); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal("application/json", response.Content.Headers.ContentType.MediaType); + Assert.Equal("null", content); + } + + // If the object is a string, it will get formatted as JSON. NOT as text/plain. + [Fact] + public async Task JsonResult_String() + { + // Arrange + var server = TestServer.Create(_provider, _app); + var client = server.CreateClient(); + + var url = "http://localhost/JsonResult/String"; + + var request = new HttpRequestMessage(HttpMethod.Get, url); + + // Act + var response = await client.SendAsync(request); + var content = await response.Content.ReadAsStringAsync(); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal("application/json", response.Content.Headers.ContentType.MediaType); + Assert.Equal("\"hello\"", content); } [Theory] diff --git a/test/WebSites/BasicWebSite/Controllers/JsonResultController.cs b/test/WebSites/BasicWebSite/Controllers/JsonResultController.cs index caf7475446..a88d498fc5 100644 --- a/test/WebSites/BasicWebSite/Controllers/JsonResultController.cs +++ b/test/WebSites/BasicWebSite/Controllers/JsonResultController.cs @@ -36,5 +36,10 @@ public JsonResult Null() { return Json(null); } + + public JsonResult String() + { + return Json("hello"); + } } } \ No newline at end of file