Making FormatFilter a Service and some test changes

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)
         {

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
         /// <summary>
         /// Initializes an instance of <see cref="FormatFilter"/>.
         /// </summary>
-        /// <param name="options"><see cref="MvcOptions"/>.</param>
+        /// <param name="options">The <see cref="IOptions{MvcOptions}"/></param>
-        public FormatFilter(MvcOptions options, ActionContext actionContext)
+        /// <param name="actionContext">The <see cref="IScopedInstance{ActionContext}"/></param>
+        public FormatFilter(IOptions<MvcOptions> options, IScopedInstance<ActionContext> 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);
         }
 
         /// <summary>
-        /// format value in the current request. <c>null</c> if format not present in the current request.
+        /// Format value in the current request. <c>null</c> if format not present in the current request.
         /// </summary>
-        public string Format { get; private set; }
+        public string Format { get; }
 
         /// <summary>
         /// <see cref="MediaTypeHeaderValue"/> for the format value in the current request.
         /// </summary>
-        public MediaTypeHeaderValue ContentType { get; private set; }
+        public MediaTypeHeaderValue ContentType { get; }
 
         /// <summary>
         /// <c>true</c> if the current <see cref="FormatFilter"/> is active and will execute. 
         /// </summary>
-        public bool IsActive { get; private set; }
+        public bool IsActive { get; }
 
         /// <summary>
         /// As a <see cref="IResourceFilter"/>, 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<IApiResponseMetadataProvider>();
+            var contentTypes = new List<MediaTypeHeaderValue>();
+
+            foreach (var filter in responseTypeFilters)
             {
-                var responseTypeFilters = context.Filters.OfType<IApiResponseMetadataProvider>();
+                filter.SetContentTypes(contentTypes);
-                var contentTypes = new List<MediaTypeHeaderValue>();
+            }
 
-                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);
+                    context.Result = new HttpNotFoundResult();
-                }
-
-                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();
-                    }
                 }
             }
         }

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
         /// <returns>An instance of <see cref="FormatFilter"/></returns>
         public IFilter CreateInstance([NotNull] IServiceProvider serviceProvider)
         {
-            var options = serviceProvider.GetRequiredService<IOptions<MvcOptions>>();
+            return serviceProvider.GetRequiredService<FormatFilter>();
-            var actionContext = serviceProvider.GetRequiredService<IScopedInstance<ActionContext>>();
-            return new FormatFilter(options.Options, actionContext.Value);
         }
     }
 }

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;

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
 {

src/Microsoft.AspNet.Mvc/MvcServices.cs

@@ -75,7 +75,7 @@ namespace Microsoft.AspNet.Mvc
 
             yield return describe.Transient<INestedProvider<FilterProviderContext>, DefaultFilterProvider>();
 
-            yield return describe.Transient<IFilterFactory, FormatFilterAttribute>();
+            yield return describe.Transient<FormatFilter, FormatFilter>();
 
             // Dataflow - ModelBinding, Validation and Formatting
 

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, 
+            string format,
-            FormatSource place, 
+            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 = new FormatFilter(mockObjects.IOptions, mockObjects.ScopedInstance);
-            var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider);
 
             // Act
             filter.OnResourceExecuting(resourceExecutingContext);
@@ -55,7 +54,7 @@ namespace Microsoft.AspNet.Mvc
 
             // Act
             filter.OnResultExecuting(resultExecutingContext);
-            
+
             // Assert
             var objectResult = Assert.IsType<ObjectResult>(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>();
-            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, 
+                ac,
-                new IFilter[] { }, 
+                new IFilter[] { },
                 new ObjectResult("Hello!"),
                 controller: new object());
-            
+
             var resourceExecutingContext = new ResourceExecutingContext(
                 ac,
                 new IFilter[] { });
 
-            var filterAttribute = new FormatFilterAttribute();
+            var filter = new FormatFilter(mockObjects.IOptions, mockObjects.ScopedInstance);
-            var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider);
 
             // 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, 
+            string format,
-            FormatSource place, 
+            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<IOptions<MvcOptions>>();
+            mockObjects.MockOptions.FormatterMappings.SetMediaTypeMappingForFormat(
-            options.Options.FormatterMappings.SetMediaTypeMappingForFormat(
+                format,
-                format, 
                 MediaTypeHeaderValue.Parse(contentType));
 
-            var filterAttribute = new FormatFilterAttribute();
+            var filter = new FormatFilter(mockObjects.IOptions, mockObjects.ScopedInstance);
-            var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider);
 
             // Act
             filter.OnResourceExecuting(resourceExecutingContext);
             filter.OnResultExecuting(resultExecutingContext);
-            
+
             // Assert
             var objectResult = Assert.IsType<ObjectResult>(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 = new FormatFilter(mockObjects.IOptions, mockObjects.ScopedInstance);
-            var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider);
 
             // 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 = new FormatFilter(mockObjects.IOptions, mockObjects.ScopedInstance);
-            var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider);
 
             // 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 = new FormatFilter(mockObjects.IOptions, mockObjects.ScopedInstance);
-            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<IOptions<MvcOptions>>();
-            options.Options.FormatterMappings.SetMediaTypeMappingForFormat(
-                "xml", 
-                MediaTypeHeaderValue.Parse("application/xml"));
-            
-            var filterAttribute = new FormatFilterAttribute();            
-            var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider);
 
             // 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<IOptions<MvcOptions>>();
+
-            options.Options.FormatterMappings.SetMediaTypeMappingForFormat(
+            mockObjects.MockOptions.FormatterMappings.SetMediaTypeMappingForFormat(
                 "xml",
                 MediaTypeHeaderValue.Parse("application/xml"));
 
-            var filterAttribute = new FormatFilterAttribute();            
+            var filter = new FormatFilter(mockObjects.IOptions, mockObjects.ScopedInstance);
-            var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider);
 
             // 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<IOptions<MvcOptions>>();
+
-            options.Options.FormatterMappings.SetMediaTypeMappingForFormat(
+            mockObjects.MockOptions.FormatterMappings.SetMediaTypeMappingForFormat(
                 "xml",
                 MediaTypeHeaderValue.Parse("application/xml;version=1"));
 
-            var filterAttribute = new FormatFilterAttribute();            
+            var filter = new FormatFilter(mockObjects.IOptions, mockObjects.ScopedInstance);
-            var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider);
 
             // 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<IOptions<MvcOptions>>();
+
-            options.Options.FormatterMappings.SetMediaTypeMappingForFormat(
+            mockObjects.MockOptions.FormatterMappings.SetMediaTypeMappingForFormat(
                 "xml",
                 MediaTypeHeaderValue.Parse("application/xml"));
 
-            var filterAttribute = new FormatFilterAttribute();            
+            var filter = new FormatFilter(mockObjects.IOptions, mockObjects.ScopedInstance);
-            var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider);
+
-            
             // Act
             filter.OnResourceExecuting(resourceExecutingContext);
 
             // Assert
             var result = Assert.IsType<HttpNotFoundResult>(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 = new FormatFilter(mockObjects.IOptions, mockObjects.ScopedInstance);
-            var filter = (FormatFilter)filterAttribute.CreateInstance(mockObjects.MockServiceProvider);
 
             // 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<ActionContext> ScopedInstance { get; private set; }
+            public IOptions<MvcOptions> IOptions { get; private set; }
+
             public MockObjects(string format = null, FormatSource? place = null)
             {
                 var httpContext = new Mock<HttpContext>();
-                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> httpContext, 
+                Mock<HttpContext> httpContext,
-                string format, 
+                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> httpContext,
                 string format = null,
                 FormatSource? place = null)
             {
                 // Setup options on mock service provider
-                var options = new MvcOptions();
+                MockOptions = new MvcOptions();
-                //MvcOptionsSetup.ConfigureMvc(options);
 
                 // Set up default output formatters.
-                options.OutputFormatters.Add(new HttpNoContentOutputFormatter());
+                MockOptions.OutputFormatters.Add(new HttpNoContentOutputFormatter());
-                options.OutputFormatters.Add(new StringOutputFormatter());
+                MockOptions.OutputFormatters.Add(new StringOutputFormatter());
-                options.OutputFormatters.Add(new JsonOutputFormatter());
+                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<IOptions<MvcOptions>>();
-                mvcOptions.Setup(o => o.Options).Returns(options);
+                mvcOptions.Setup(o => o.Options).Returns(MockOptions);
-
+                IOptions = mvcOptions.Object;
-                var serviceProvider = new Mock<IServiceProvider>();
-                serviceProvider
-                    .Setup(s => s.GetService(It.Is<Type>(t => t == typeof(IOptions<MvcOptions>))))
-                    .Returns(mvcOptions.Object);
 
                 // Setup MVC services on mock service provider
                 MockActionContext = CreateMockActionContext(httpContext, format, place);
                 var scopedInstance = new Mock<IScopedInstance<ActionContext>>();
                 scopedInstance.Setup(s => s.Value).Returns(MockActionContext);
-
+                ScopedInstance = scopedInstance.Object;
-                serviceProvider
-                    .Setup(s => s.GetService(It.Is<Type>(t => t == typeof(IScopedInstance<ActionContext>))))
-                    .Returns(scopedInstance.Object);
-
-                return serviceProvider.Object;
             }
         }
 #endif