From 514460b8018ff84158196e6421652538297fa10c Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Thu, 29 Jan 2015 14:06:47 -0800 Subject: [PATCH] Making FormatFilter a Service and some test changes --- .../Controllers/FormatFilterController.cs | 2 +- .../Filters/FormatFilter.cs | 48 +++--- .../Filters/FormatFilterAttribute.cs | 5 +- .../Filters/ProducesAttribute.cs | 1 - src/Microsoft.AspNet.Mvc.Core/MvcOptions.cs | 1 - src/Microsoft.AspNet.Mvc/MvcServices.cs | 2 +- .../Filters/FormatFilterTest.cs | 161 ++++++------------ 7 files changed, 84 insertions(+), 136 deletions(-) diff --git a/samples/MvcSample.Web/Controllers/FormatFilterController.cs b/samples/MvcSample.Web/Controllers/FormatFilterController.cs index 5debd2d10b..37077204f6 100644 --- a/samples/MvcSample.Web/Controllers/FormatFilterController.cs +++ b/samples/MvcSample.Web/Controllers/FormatFilterController.cs @@ -12,7 +12,7 @@ namespace MvcSample.Web.Controllers { return new Product() { SampleInt = id }; } - + [Produces("application/json", "text/json")] public Product ProducesMethod(int id) { diff --git a/src/Microsoft.AspNet.Mvc.Core/Filters/FormatFilter.cs b/src/Microsoft.AspNet.Mvc.Core/Filters/FormatFilter.cs index b2043a4dde..aa110cb013 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Filters/FormatFilter.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Filters/FormatFilter.cs @@ -4,6 +4,8 @@ using System.Collections.Generic; using System.Linq; using Microsoft.AspNet.Mvc.Description; +using Microsoft.Framework.DependencyInjection; +using Microsoft.Framework.OptionsModel; using Microsoft.Net.Http.Headers; namespace Microsoft.AspNet.Mvc @@ -17,11 +19,12 @@ namespace Microsoft.AspNet.Mvc /// /// Initializes an instance of . /// - /// . - public FormatFilter(MvcOptions options, ActionContext actionContext) + /// The + /// The + public FormatFilter(IOptions options, IScopedInstance actionContext) { IsActive = true; - Format = GetFormat(actionContext); + Format = GetFormat(actionContext.Value); if (string.IsNullOrEmpty(Format)) { @@ -29,23 +32,23 @@ namespace Microsoft.AspNet.Mvc return; } - ContentType = options.FormatterMappings.GetMediaTypeMappingForFormat(Format); + ContentType = options.Options.FormatterMappings.GetMediaTypeMappingForFormat(Format); } /// - /// format value in the current request. null if format not present in the current request. + /// Format value in the current request. null if format not present in the current request. /// - public string Format { get; private set; } + public string Format { get; } /// /// for the format value in the current request. /// - public MediaTypeHeaderValue ContentType { get; private set; } + public MediaTypeHeaderValue ContentType { get; } /// /// true if the current is active and will execute. /// - public bool IsActive { get; private set; } + public bool IsActive { get; } /// /// As a , this filter looks at the request and rejects it before going ahead if @@ -64,25 +67,24 @@ namespace Microsoft.AspNet.Mvc { // no contentType exists for the format, return 404 context.Result = new HttpNotFoundResult(); + return; } - else + + var responseTypeFilters = context.Filters.OfType(); + var contentTypes = new List(); + + foreach (var filter in responseTypeFilters) { - var responseTypeFilters = context.Filters.OfType(); - var contentTypes = new List(); + filter.SetContentTypes(contentTypes); + } - foreach (var filter in responseTypeFilters) + if (contentTypes.Count != 0) + { + // We need to check if the action can generate the content type the user asked for. If it cannot, exit + // here with not found result. + if (!contentTypes.Any(c => ContentType.IsSubsetOf(c))) { - filter.SetContentTypes(contentTypes); - } - - if (contentTypes.Count != 0) - { - // There is no IApiResponseMetadataProvider to generate the content type user asked for. We have to - // exit here with not found result. - if (!contentTypes.Any(c => ContentType.IsSubsetOf(c))) - { - context.Result = new HttpNotFoundResult(); - } + context.Result = new HttpNotFoundResult(); } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/Filters/FormatFilterAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/Filters/FormatFilterAttribute.cs index 8836373227..4bae9551cd 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Filters/FormatFilterAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Filters/FormatFilterAttribute.cs @@ -3,7 +3,6 @@ using System; using Microsoft.Framework.DependencyInjection; -using Microsoft.Framework.OptionsModel; namespace Microsoft.AspNet.Mvc { @@ -21,9 +20,7 @@ namespace Microsoft.AspNet.Mvc /// An instance of public IFilter CreateInstance([NotNull] IServiceProvider serviceProvider) { - var options = serviceProvider.GetRequiredService>(); - var actionContext = serviceProvider.GetRequiredService>(); - return new FormatFilter(options.Options, actionContext.Value); + return serviceProvider.GetRequiredService(); } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/Filters/ProducesAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/Filters/ProducesAttribute.cs index 2a12120ec5..2e89971978 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Filters/ProducesAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Filters/ProducesAttribute.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; -using System.Linq; using System.Collections.Generic; using System.Linq; using Microsoft.AspNet.Mvc.Core; diff --git a/src/Microsoft.AspNet.Mvc.Core/MvcOptions.cs b/src/Microsoft.AspNet.Mvc.Core/MvcOptions.cs index 6eda47c88e..8bb53e32cc 100644 --- a/src/Microsoft.AspNet.Mvc.Core/MvcOptions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/MvcOptions.cs @@ -7,7 +7,6 @@ using Microsoft.AspNet.Mvc.ApplicationModels; using Microsoft.AspNet.Mvc.Core; using Microsoft.AspNet.Mvc.OptionDescriptors; using Microsoft.AspNet.Mvc.ModelBinding; -using Microsoft.AspNet.Mvc.HeaderValueAbstractions; namespace Microsoft.AspNet.Mvc { diff --git a/src/Microsoft.AspNet.Mvc/MvcServices.cs b/src/Microsoft.AspNet.Mvc/MvcServices.cs index 8ce05b618e..114fba6b45 100644 --- a/src/Microsoft.AspNet.Mvc/MvcServices.cs +++ b/src/Microsoft.AspNet.Mvc/MvcServices.cs @@ -75,7 +75,7 @@ namespace Microsoft.AspNet.Mvc yield return describe.Transient, DefaultFilterProvider>(); - yield return describe.Transient(); + yield return describe.Transient(); // Dataflow - ModelBinding, Validation and Formatting diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Filters/FormatFilterTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Filters/FormatFilterTest.cs index 001c0e8cc7..cd623b04d8 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Filters/FormatFilterTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Filters/FormatFilterTest.cs @@ -33,8 +33,8 @@ namespace Microsoft.AspNet.Mvc [InlineData("json", FormatSource.QueryData, "application/json")] [InlineData("json", FormatSource.RouteAndQueryData, "application/json")] public void FormatFilter_ContextContainsFormat_DefaultFormat( - string format, - FormatSource place, + string format, + FormatSource place, string contentType) { // Arrange @@ -44,8 +44,7 @@ namespace Microsoft.AspNet.Mvc var resultExecutingContext = mockObjects.CreateResultExecutingContext(); var resourceExecutingContext = mockObjects.CreateResourceExecutingContext(new IFilter[] { }); - var filterAttribute = new FormatFilterAttribute(); - var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider); + var filter = new FormatFilter(mockObjects.IOptions, mockObjects.ScopedInstance); // Act filter.OnResourceExecuting(resourceExecutingContext); @@ -55,7 +54,7 @@ namespace Microsoft.AspNet.Mvc // Act filter.OnResultExecuting(resultExecutingContext); - + // Assert var objectResult = Assert.IsType(resultExecutingContext.Result); Assert.Equal(1, objectResult.ContentTypes.Count); @@ -70,11 +69,7 @@ namespace Microsoft.AspNet.Mvc // Arrange var mediaType = MediaTypeHeaderValue.Parse("application/json"); var mockObjects = new MockObjects("json", FormatSource.RouteData); - var httpContext = new Mock(); - httpContext - .Setup(c => c.RequestServices) - .Returns(mockObjects.MockServiceProvider); // Query contains xml httpContext.Setup(c => c.Request.Query.ContainsKey("format")).Returns(true); @@ -87,17 +82,16 @@ namespace Microsoft.AspNet.Mvc var ac = new ActionContext(httpContext.Object, data, new ActionDescriptor()); var resultExecutingContext = new ResultExecutingContext( - ac, - new IFilter[] { }, + ac, + new IFilter[] { }, new ObjectResult("Hello!"), controller: new object()); - + var resourceExecutingContext = new ResourceExecutingContext( ac, new IFilter[] { }); - var filterAttribute = new FormatFilterAttribute(); - var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider); + var filter = new FormatFilter(mockObjects.IOptions, mockObjects.ScopedInstance); // Act filter.OnResourceExecuting(resourceExecutingContext); @@ -114,29 +108,27 @@ namespace Microsoft.AspNet.Mvc [InlineData("foo", FormatSource.QueryData, "application/foo")] [InlineData("foo", FormatSource.RouteAndQueryData, "application/foo")] public void FormatFilter_ContextContainsFormat_Custom( - string format, - FormatSource place, + string format, + FormatSource place, string contentType) { // Arrange var mediaType = MediaTypeHeaderValue.Parse(contentType); - + var mockObjects = new MockObjects(format, place); var resultExecutingContext = mockObjects.CreateResultExecutingContext(); var resourceExecutingContext = mockObjects.CreateResourceExecutingContext(new IFilter[] { }); - var options = mockObjects.MockServiceProvider.GetService>(); - options.Options.FormatterMappings.SetMediaTypeMappingForFormat( - format, + mockObjects.MockOptions.FormatterMappings.SetMediaTypeMappingForFormat( + format, MediaTypeHeaderValue.Parse(contentType)); - var filterAttribute = new FormatFilterAttribute(); - var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider); + var filter = new FormatFilter(mockObjects.IOptions, mockObjects.ScopedInstance); // Act filter.OnResourceExecuting(resourceExecutingContext); filter.OnResultExecuting(resultExecutingContext); - + // Assert var objectResult = Assert.IsType(resultExecutingContext.Result); Assert.Equal(1, objectResult.ContentTypes.Count); @@ -154,8 +146,7 @@ namespace Microsoft.AspNet.Mvc var mockObjects = new MockObjects(format, place); var resourceExecutingContext = mockObjects.CreateResourceExecutingContext(new IFilter[] { }); - var filterAttribute = new FormatFilterAttribute(); - var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider); + var filter = new FormatFilter(mockObjects.IOptions, mockObjects.ScopedInstance); // Act filter.OnResourceExecuting(resourceExecutingContext); @@ -172,8 +163,7 @@ namespace Microsoft.AspNet.Mvc var mockObjects = new MockObjects(); var resourceExecutingContext = mockObjects.CreateResourceExecutingContext(new IFilter[] { }); - var filterAttribute = new FormatFilterAttribute(); - var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider); + var filter = new FormatFilter(mockObjects.IOptions, mockObjects.ScopedInstance); // Act filter.OnResourceExecuting(resourceExecutingContext); @@ -195,30 +185,7 @@ namespace Microsoft.AspNet.Mvc var mockObjects = new MockObjects(format, place); var resourceExecutingContext = mockObjects.CreateResourceExecutingContext(new IFilter[] { produces }); - var filterAttribute = new FormatFilterAttribute(); - var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider); - - // Act - filter.OnResourceExecuting(resourceExecutingContext); - - // Assert - Assert.Null(resourceExecutingContext.Result); - } - - [Fact] - public void FormatFilter_LessSpecificThan_Produces() - { - // Arrange - var produces = new ProducesAttribute("application/xml;version=1", new string [] { }); - var mockObjects = new MockObjects("xml", FormatSource.RouteData); - var resourceExecutingContext = mockObjects.CreateResourceExecutingContext(new IFilter[] { produces }); - var options = mockObjects.MockServiceProvider.GetService>(); - options.Options.FormatterMappings.SetMediaTypeMappingForFormat( - "xml", - MediaTypeHeaderValue.Parse("application/xml")); - - var filterAttribute = new FormatFilterAttribute(); - var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider); + var filter = new FormatFilter(mockObjects.IOptions, mockObjects.ScopedInstance); // Act filter.OnResourceExecuting(resourceExecutingContext); @@ -228,20 +195,18 @@ namespace Microsoft.AspNet.Mvc } [Fact] - public void FormatFilter_LessSpecificThan_Produces_Wildcard() + public void FormatFilter_LessSpecificThan_Produces() { // Arrange - var produces = new ProducesAttribute("application/*", new string[] { }); - + var produces = new ProducesAttribute("application/xml;version=1", new string[] { }); var mockObjects = new MockObjects("xml", FormatSource.RouteData); var resourceExecutingContext = mockObjects.CreateResourceExecutingContext(new IFilter[] { produces }); - var options = mockObjects.MockServiceProvider.GetService>(); - options.Options.FormatterMappings.SetMediaTypeMappingForFormat( + + mockObjects.MockOptions.FormatterMappings.SetMediaTypeMappingForFormat( "xml", MediaTypeHeaderValue.Parse("application/xml")); - var filterAttribute = new FormatFilterAttribute(); - var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider); + var filter = new FormatFilter(mockObjects.IOptions, mockObjects.ScopedInstance); // Act filter.OnResourceExecuting(resourceExecutingContext); @@ -257,13 +222,12 @@ namespace Microsoft.AspNet.Mvc var produces = new ProducesAttribute("application/xml", new string[] { }); var mockObjects = new MockObjects("xml", FormatSource.RouteData); var resourceExecutingContext = mockObjects.CreateResourceExecutingContext(new IFilter[] { produces }); - var options = mockObjects.MockServiceProvider.GetService>(); - options.Options.FormatterMappings.SetMediaTypeMappingForFormat( + + mockObjects.MockOptions.FormatterMappings.SetMediaTypeMappingForFormat( "xml", MediaTypeHeaderValue.Parse("application/xml;version=1")); - var filterAttribute = new FormatFilterAttribute(); - var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider); + var filter = new FormatFilter(mockObjects.IOptions, mockObjects.ScopedInstance); // Act filter.OnResourceExecuting(resourceExecutingContext); @@ -284,21 +248,20 @@ namespace Microsoft.AspNet.Mvc var produces = new ProducesAttribute("application/xml", new string[] { "application/foo", "text/bar" }); var mockObjects = new MockObjects(format, place); var resourceExecutingContext = mockObjects.CreateResourceExecutingContext(new IFilter[] { produces }); - var options = mockObjects.MockServiceProvider.GetService>(); - options.Options.FormatterMappings.SetMediaTypeMappingForFormat( + + mockObjects.MockOptions.FormatterMappings.SetMediaTypeMappingForFormat( "xml", MediaTypeHeaderValue.Parse("application/xml")); - var filterAttribute = new FormatFilterAttribute(); - var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider); - + var filter = new FormatFilter(mockObjects.IOptions, mockObjects.ScopedInstance); + // Act filter.OnResourceExecuting(resourceExecutingContext); // Assert var result = Assert.IsType(resourceExecutingContext.Result); } - + [Theory] [InlineData("", FormatSource.RouteData)] [InlineData(null, FormatSource.QueryData)] @@ -311,8 +274,7 @@ namespace Microsoft.AspNet.Mvc // Arrange var mockObjects = new MockObjects(format, place); var resourceExecutingContext = mockObjects.CreateResourceExecutingContext(new IFilter[] { }); - var filterAttribute = new FormatFilterAttribute(); - var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider); + var filter = new FormatFilter(mockObjects.IOptions, mockObjects.ScopedInstance); // Act filter.OnResourceExecuting(resourceExecutingContext); @@ -323,7 +285,7 @@ namespace Microsoft.AspNet.Mvc [Theory] [InlineData("json", FormatSource.RouteData, true)] - [InlineData("json", FormatSource.QueryData, true )] + [InlineData("json", FormatSource.QueryData, true)] [InlineData("", FormatSource.RouteAndQueryData, false)] [InlineData(null, FormatSource.RouteAndQueryData, false)] public void FormatFilter_IsActive( @@ -335,14 +297,14 @@ namespace Microsoft.AspNet.Mvc var mockObjects = new MockObjects(format, place); var resultExecutingContext = mockObjects.CreateResultExecutingContext(); var filterAttribute = new FormatFilterAttribute(); - var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider); + var filter = new FormatFilter(mockObjects.IOptions, mockObjects.ScopedInstance); // Act and Assert Assert.Equal(expected, filter.IsActive); } private static void AssertMediaTypesEqual( - MediaTypeHeaderValue expectedMediaType, + MediaTypeHeaderValue expectedMediaType, MediaTypeHeaderValue actualMediaType) { Assert.Equal(expectedMediaType.MediaType, actualMediaType.MediaType); @@ -357,25 +319,22 @@ namespace Microsoft.AspNet.Mvc } } - public class MockObjects + private class MockObjects { - public IServiceProvider MockServiceProvider { get; private set; } + public MvcOptions MockOptions { get; private set; } public HttpContext MockHttpContext { get; private set; } public ActionContext MockActionContext { get; private set; } - + + public IScopedInstance ScopedInstance { get; private set; } + public IOptions IOptions { get; private set; } + public MockObjects(string format = null, FormatSource? place = null) { var httpContext = new Mock(); - MockServiceProvider = CreateMockServiceProvider(httpContext, format, place); - - httpContext - .Setup(c => c.RequestServices) - .Returns(MockServiceProvider); - httpContext.Setup(c => c.Request.Query.ContainsKey("format")).Returns(false); - MockHttpContext = httpContext.Object; - //MockActionContext = CreateMockActionContext(httpContext, format, place); + + Initialize(httpContext, format, place); } public ResourceExecutingContext CreateResourceExecutingContext(IFilter[] filters) @@ -396,8 +355,8 @@ namespace Microsoft.AspNet.Mvc } private ActionContext CreateMockActionContext( - Mock httpContext, - string format, + Mock httpContext, + string format, FormatSource? place) { var data = new RouteData(); @@ -421,41 +380,33 @@ namespace Microsoft.AspNet.Mvc return new ActionContext(httpContext.Object, data, new ActionDescriptor()); } - private IServiceProvider CreateMockServiceProvider( + private void Initialize( Mock httpContext, string format = null, FormatSource? place = null) { // Setup options on mock service provider - var options = new MvcOptions(); - //MvcOptionsSetup.ConfigureMvc(options); + MockOptions = new MvcOptions(); // Set up default output formatters. - options.OutputFormatters.Add(new HttpNoContentOutputFormatter()); - options.OutputFormatters.Add(new StringOutputFormatter()); - options.OutputFormatters.Add(new JsonOutputFormatter()); + MockOptions.OutputFormatters.Add(new HttpNoContentOutputFormatter()); + MockOptions.OutputFormatters.Add(new StringOutputFormatter()); + MockOptions.OutputFormatters.Add(new JsonOutputFormatter()); // Set up default mapping for json extensions to content type - options.FormatterMappings.SetMediaTypeMappingForFormat("json", MediaTypeHeaderValue.Parse("application/json")); + MockOptions.FormatterMappings.SetMediaTypeMappingForFormat( + "json", + MediaTypeHeaderValue.Parse("application/json")); var mvcOptions = new Mock>(); - mvcOptions.Setup(o => o.Options).Returns(options); - - var serviceProvider = new Mock(); - serviceProvider - .Setup(s => s.GetService(It.Is(t => t == typeof(IOptions)))) - .Returns(mvcOptions.Object); + mvcOptions.Setup(o => o.Options).Returns(MockOptions); + IOptions = mvcOptions.Object; // Setup MVC services on mock service provider MockActionContext = CreateMockActionContext(httpContext, format, place); var scopedInstance = new Mock>(); scopedInstance.Setup(s => s.Value).Returns(MockActionContext); - - serviceProvider - .Setup(s => s.GetService(It.Is(t => t == typeof(IScopedInstance)))) - .Returns(scopedInstance.Object); - - return serviceProvider.Object; + ScopedInstance = scopedInstance.Object; } } #endif