diff --git a/src/Microsoft.AspNet.Mvc.Core/DependencyInjection/MvcCoreMvcCoreBuilderExtensions.cs b/src/Microsoft.AspNet.Mvc.Core/DependencyInjection/MvcCoreMvcCoreBuilderExtensions.cs index e4222c898b..b8359abbb5 100644 --- a/src/Microsoft.AspNet.Mvc.Core/DependencyInjection/MvcCoreMvcCoreBuilderExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/DependencyInjection/MvcCoreMvcCoreBuilderExtensions.cs @@ -62,7 +62,7 @@ namespace Microsoft.Extensions.DependencyInjection // Internal for testing. internal static void AddFormatterMappingsServices(IServiceCollection services) { - services.TryAddTransient(); + services.TryAddSingleton(); } public static IMvcCoreBuilder AddAuthorization(this IMvcCoreBuilder builder) diff --git a/src/Microsoft.AspNet.Mvc.Core/Formatters/FormatFilter.cs b/src/Microsoft.AspNet.Mvc.Core/Formatters/FormatFilter.cs index 8dd2dd8f03..d72216124b 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Formatters/FormatFilter.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Formatters/FormatFilter.cs @@ -6,7 +6,6 @@ using System.Collections.Generic; using System.Linq; using Microsoft.AspNet.Mvc.ApiExplorer; using Microsoft.AspNet.Mvc.Filters; -using Microsoft.AspNet.Mvc.Infrastructure; using Microsoft.Extensions.OptionsModel; using Microsoft.Net.Http.Headers; @@ -18,39 +17,36 @@ namespace Microsoft.AspNet.Mvc.Formatters /// public class FormatFilter : IFormatFilter, IResourceFilter, IResultFilter { + private readonly MvcOptions _options; + /// /// Initializes an instance of . /// /// The - /// The - public FormatFilter(IOptions options, IActionContextAccessor actionContextAccessor) + public FormatFilter(IOptions options) { - IsActive = true; - Format = GetFormat(actionContextAccessor.ActionContext); - - if (string.IsNullOrEmpty(Format)) - { - IsActive = false; - return; - } - - ContentType = options.Value.FormatterMappings.GetMediaTypeMappingForFormat(Format); + _options = options.Value; } - /// - /// Format value in the current request. null if format not present in the current request. - /// - public string Format { get; } + /// + public string GetFormat(ActionContext context) + { + object obj; + if (context.RouteData.Values.TryGetValue("format", out obj)) + { + // null and string.Empty are equivalent for route values. + var routeValue = obj?.ToString(); + return string.IsNullOrEmpty(routeValue) ? null : routeValue; + } - /// - /// for the format value in the current request. - /// - public MediaTypeHeaderValue ContentType { get; } + var query = context.HttpContext.Request.Query["format"]; + if (query.Count > 0) + { + return query.ToString(); + } - /// - /// true if the current is active and will execute. - /// - public bool IsActive { get; } + return null; + } /// /// As a , this filter looks at the request and rejects it before going ahead if @@ -65,13 +61,15 @@ namespace Microsoft.AspNet.Mvc.Formatters throw new ArgumentNullException(nameof(context)); } - if (!IsActive) + var format = GetFormat(context); + if (format == null) { // no format specified by user, so the filter is muted return; } - if (ContentType == null) + var contentType = _options.FormatterMappings.GetMediaTypeMappingForFormat(format); + if (contentType == null) { // no contentType exists for the format, return 404 context.Result = new HttpNotFoundResult(); @@ -93,7 +91,7 @@ namespace Microsoft.AspNet.Mvc.Formatters // request's format and IApiResponseMetadataProvider-provided content types similarly to an Accept // header and an output formatter's SupportedMediaTypes: Confirm action supports a more specific media // type than requested e.g. OK if "text/*" requested and action supports "text/plain". - if (!supportedMediaTypes.Any(contentType => contentType.IsSubsetOf(ContentType))) + if (!supportedMediaTypes.Any(c => c.IsSubsetOf(contentType))) { context.Result = new HttpNotFoundResult(); } @@ -116,43 +114,25 @@ namespace Microsoft.AspNet.Mvc.Formatters throw new ArgumentNullException(nameof(context)); } - if (!IsActive) + var format = GetFormat(context); + if (format == null) { - return; // no format specified by user, so the filter is muted + // no format specified by user, so the filter is muted + return; } var objectResult = context.Result as ObjectResult; if (objectResult != null) { + var contentType = _options.FormatterMappings.GetMediaTypeMappingForFormat(format); objectResult.ContentTypes.Clear(); - objectResult.ContentTypes.Add(ContentType); + objectResult.ContentTypes.Add(contentType); } } /// public void OnResultExecuted(ResultExecutedContext context) { - if (context == null) - { - throw new ArgumentNullException(nameof(context)); - } - } - - private string GetFormat(ActionContext context) - { - object format = null; - - if (!context.RouteData.Values.TryGetValue("format", out format)) - { - format = context.HttpContext.Request.Query["format"]; - } - - if (format != null) - { - return format.ToString(); - } - - return null; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/Formatters/IFormatFilter.cs b/src/Microsoft.AspNet.Mvc.Core/Formatters/IFormatFilter.cs index 78377cde4a..301b7ea77b 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Formatters/IFormatFilter.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Formatters/IFormatFilter.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using Microsoft.AspNet.Mvc.Filters; -using Microsoft.Net.Http.Headers; namespace Microsoft.AspNet.Mvc.Formatters { @@ -12,18 +11,10 @@ namespace Microsoft.AspNet.Mvc.Formatters public interface IFormatFilter : IFilterMetadata { /// - /// format value in the current request. null if format not present in the current request. + /// Gets the format value for the request associated with the provided . /// - string Format { get; } - - /// - /// for the format value in the current request. - /// - MediaTypeHeaderValue ContentType { get; } - - /// - /// true if the current is active and will execute. - /// - bool IsActive { get; } + /// The associated with the current request. + /// A format value, or null if a format cannot be determined for the request. + string GetFormat(ActionContext context); } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/ProducesAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/ProducesAttribute.cs index 1f06e711c4..a969c3fbb8 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ProducesAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ProducesAttribute.cs @@ -67,7 +67,7 @@ namespace Microsoft.AspNet.Mvc { // Check if there are any IFormatFilter in the pipeline, and if any of them is active. If there is one, // do not override the content type value. - if (context.Filters.OfType().All(f => !f.IsActive)) + if (context.Filters.OfType().All(f => f.GetFormat(context) == null)) { SetContentTypes(objectResult.ContentTypes); } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/FormatFilterTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/FormatFilterTest.cs index d4fd11e176..b95aafdbce 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/FormatFilterTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/FormatFilterTest.cs @@ -38,7 +38,7 @@ namespace Microsoft.AspNet.Mvc.Formatters var resultExecutingContext = mockObjects.CreateResultExecutingContext(); var resourceExecutingContext = mockObjects.CreateResourceExecutingContext(new IFilterMetadata[] { }); - var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor); + var filter = new FormatFilter(mockObjects.OptionsManager); // Act filter.OnResourceExecuting(resourceExecutingContext); @@ -85,7 +85,7 @@ namespace Microsoft.AspNet.Mvc.Formatters ac, new IFilterMetadata[] { }); - var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor); + var filter = new FormatFilter(mockObjects.OptionsManager); // Act filter.OnResourceExecuting(resourceExecutingContext); @@ -117,7 +117,7 @@ namespace Microsoft.AspNet.Mvc.Formatters format, MediaTypeHeaderValue.Parse(contentType)); - var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor); + var filter = new FormatFilter(mockObjects.OptionsManager); // Act filter.OnResourceExecuting(resourceExecutingContext); @@ -140,7 +140,7 @@ namespace Microsoft.AspNet.Mvc.Formatters var mockObjects = new MockObjects(format, place); var resourceExecutingContext = mockObjects.CreateResourceExecutingContext(new IFilterMetadata[] { }); - var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor); + var filter = new FormatFilter(mockObjects.OptionsManager); // Act filter.OnResourceExecuting(resourceExecutingContext); @@ -157,7 +157,7 @@ namespace Microsoft.AspNet.Mvc.Formatters var mockObjects = new MockObjects(); var resourceExecutingContext = mockObjects.CreateResourceExecutingContext(new IFilterMetadata[] { }); - var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor); + var filter = new FormatFilter(mockObjects.OptionsManager); // Act filter.OnResourceExecuting(resourceExecutingContext); @@ -179,7 +179,7 @@ namespace Microsoft.AspNet.Mvc.Formatters var mockObjects = new MockObjects(format, place); var resourceExecutingContext = mockObjects.CreateResourceExecutingContext(new IFilterMetadata[] { produces }); - var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor); + var filter = new FormatFilter(mockObjects.OptionsManager); // Act filter.OnResourceExecuting(resourceExecutingContext); @@ -200,7 +200,7 @@ namespace Microsoft.AspNet.Mvc.Formatters "xml", MediaTypeHeaderValue.Parse("application/xml")); - var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor); + var filter = new FormatFilter(mockObjects.OptionsManager); // Act filter.OnResourceExecuting(resourceExecutingContext); @@ -221,7 +221,7 @@ namespace Microsoft.AspNet.Mvc.Formatters "xml", MediaTypeHeaderValue.Parse("application/xml;version=1")); - var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor); + var filter = new FormatFilter(mockObjects.OptionsManager); // Act filter.OnResourceExecuting(resourceExecutingContext); @@ -247,7 +247,7 @@ namespace Microsoft.AspNet.Mvc.Formatters "xml", MediaTypeHeaderValue.Parse("application/xml")); - var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor); + var filter = new FormatFilter(mockObjects.OptionsManager); // Act filter.OnResourceExecuting(resourceExecutingContext); @@ -266,7 +266,7 @@ namespace Microsoft.AspNet.Mvc.Formatters // Arrange var mockObjects = new MockObjects(format, place); var resourceExecutingContext = mockObjects.CreateResourceExecutingContext(new IFilterMetadata[] { }); - var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor); + var filter = new FormatFilter(mockObjects.OptionsManager); // Act filter.OnResourceExecuting(resourceExecutingContext); @@ -276,23 +276,26 @@ namespace Microsoft.AspNet.Mvc.Formatters } [Theory] - [InlineData("json", FormatSource.RouteData, true)] - [InlineData("json", FormatSource.QueryData, true)] - [InlineData("", FormatSource.RouteAndQueryData, false)] - [InlineData(null, FormatSource.RouteAndQueryData, false)] - public void FormatFilter_IsActive( - string format, + [InlineData("json", FormatSource.RouteData, "json")] + [InlineData("json", FormatSource.QueryData, "json")] + [InlineData("", FormatSource.RouteAndQueryData, null)] + [InlineData(null, FormatSource.RouteAndQueryData, null)] + public void FormatFilter_GetFormat( + string input, FormatSource place, - bool expected) + string expected) { // Arrange - var mockObjects = new MockObjects(format, place); - var resultExecutingContext = mockObjects.CreateResultExecutingContext(); + var mockObjects = new MockObjects(input, place); + var context = mockObjects.CreateResultExecutingContext(); var filterAttribute = new FormatFilterAttribute(); - var filter = new FormatFilter(mockObjects.OptionsManager, mockObjects.ActionContextAccessor); + var filter = new FormatFilter(mockObjects.OptionsManager); - // Act and Assert - Assert.Equal(expected, filter.IsActive); + // Act + var format = filter.GetFormat(context); + + // Assert + Assert.Equal(expected, filter.GetFormat(context)); } private static void AssertMediaTypesEqual( @@ -317,7 +320,6 @@ namespace Microsoft.AspNet.Mvc.Formatters public HttpContext MockHttpContext { get; private set; } public ActionContext MockActionContext { get; private set; } - public IActionContextAccessor ActionContextAccessor { get; private set; } public IOptions OptionsManager { get; private set; } public MockObjects(string format = null, FormatSource? place = null) @@ -394,10 +396,6 @@ namespace Microsoft.AspNet.Mvc.Formatters // Setup MVC services on mock service provider MockActionContext = CreateMockActionContext(httpContext, format, place); - ActionContextAccessor = new ActionContextAccessor() - { - ActionContext = MockActionContext, - }; } } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ProducesAttributeTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ProducesAttributeTests.cs index 8f0834da7c..d70fa2cae1 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ProducesAttributeTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ProducesAttributeTests.cs @@ -45,8 +45,9 @@ namespace Microsoft.AspNet.Mvc.Test var producesContentAttribute = new ProducesAttribute("application/xml"); var formatFilter = new Mock(); - formatFilter.Setup(f => f.IsActive) - .Returns(false); + formatFilter + .Setup(f => f.GetFormat(It.IsAny())) + .Returns((string)null); var filters = new IFilterMetadata[] { producesContentAttribute, formatFilter.Object }; var resultExecutingContext = CreateResultExecutingContext(filters); @@ -69,8 +70,9 @@ namespace Microsoft.AspNet.Mvc.Test var producesContentAttribute = new ProducesAttribute("application/xml"); var formatFilter = new Mock(); - formatFilter.Setup(f => f.IsActive) - .Returns(true); + formatFilter + .Setup(f => f.GetFormat(It.IsAny())) + .Returns("xml"); var filters = new IFilterMetadata[] { producesContentAttribute, formatFilter.Object }; var resultExecutingContext = CreateResultExecutingContext(filters);