Adding ClearMediaTypeMappingForFormat to FormatterMappings and other PR comments

samples/MvcSample.Web/Models/Product.cs

@@ -1,4 +1,6 @@
-using System;
+// 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;
 
 namespace MvcSample.Web
 {

src/Microsoft.AspNet.Mvc.Core/ActionConstraints/ConsumesAttribute.cs

@@ -118,7 +118,7 @@ namespace Microsoft.AspNet.Mvc
                     CurrentCandidate = candidate
                 };
 
-                if (candidate.Constraints == null || candidate.Constraints.Count() == 0 ||
+                if (candidate.Constraints == null || candidate.Constraints.Count == 0 ||
                     candidate.Constraints.Any(constraint => constraint is IConsumesActionConstraint &&
                                                             constraint.Accept(tempContext)))
                 {

src/Microsoft.AspNet.Mvc.Core/Filters/FormatFilterAttribute.cs

@@ -2,13 +2,13 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
 using Microsoft.AspNet.Mvc.Description;
 using Microsoft.Framework.DependencyInjection;
 using Microsoft.Framework.OptionsModel;
 using Microsoft.Net.Http.Headers;
-using System.Collections.Generic;
 
 namespace Microsoft.AspNet.Mvc
 {
@@ -47,7 +47,7 @@ namespace Microsoft.AspNet.Mvc
                         filter.SetContentTypes(contentTypes);
                     }
 
-                    if (contentTypes.Count() != 0)
+                    if (contentTypes.Count != 0)
                     {
                         // If formatfilterContentType is not subset of any of the content types produced by 
                         // IApiResponseMetadataProviders, return 404
@@ -93,10 +93,11 @@ namespace Microsoft.AspNet.Mvc
         }
 
         /// <summary>
-        /// If the current request contains format value, returns true. It means the format filter is going to execute.
+        /// Returns <c>true</c> if the filter is active and will execute; otherwise, <c>false</c>. The filter is active
+        ///  if the current request contains format value. 
         /// </summary>
         /// <param name="context">The <see cref="FilterContext"/></param>
-        /// <returns>If the filter is active and will execute.</returns>
+        /// <returns><c>true</c> if the current request contains format value; otherwise, <c>false</c>.</returns>
         public bool IsActive(FilterContext context)
         {
             var format = GetFormat(context);
@@ -113,7 +114,12 @@ namespace Microsoft.AspNet.Mvc
                 format = context.HttpContext.Request.Query["format"];
             }
 
-            return (string)format;
+            if (format != null)
+            {
+                return format.ToString();
+            }
+
+            return null;
         }
 
         private MediaTypeHeaderValue GetContentType(string format, FilterContext context)

src/Microsoft.AspNet.Mvc.Core/Filters/IFormatFilter.cs

@@ -1,23 +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.
 
-using Microsoft.Net.Http.Headers;
-using System;
-
 namespace Microsoft.AspNet.Mvc
 {
     /// <summary>
     ///  A filter which produces a desired content type for the current request. 
     /// </summary>
-    /// <remarks>A FormatFilter decides what content type to use if a format value is present in the URL. 
-    /// </remarks>
     public interface IFormatFilter : IFilter
     {
         /// <summary>
-        /// Returns true if the filter is going to be executed for the current request.
+        /// Returns <c>true</c> if the filter will produce a content type for the current request, otherwise 
+        /// <c>false</c>.
         /// </summary>
         /// <param name="context">The <see cref="FilterContext"/></param>
-        /// <returns>If the filter will execute</returns>
+        /// <returns><c>true</c> if the filter will produce a content type for the current request; otherwise, 
+        /// <c>false</c>.</returns>
         bool IsActive(FilterContext context);
     }
 }

src/Microsoft.AspNet.Mvc.Core/FormatterMappings.cs

@@ -22,8 +22,8 @@ namespace Microsoft.AspNet.Mvc
         /// Sets mapping for the format to specified <see cref="MediaTypeHeaderValue"/>. 
         /// If the format already exists, the <see cref="MediaTypeHeaderValue"/> will be overwritten with the new value.
         /// </summary>
-        /// <param name="format">format value</param>
+        /// <param name="format">The format value.</param>
-        /// <param name="contentType">The <see cref="MediaTypeHeaderValue"/> for the format value</param>
+        /// <param name="contentType">The <see cref="MediaTypeHeaderValue"/> for the format value.</param>
         public void SetMediaTypeMappingForFormat([NotNull] string format, [NotNull] MediaTypeHeaderValue contentType)
         {
             ValidateContentType(contentType);
@@ -34,8 +34,8 @@ namespace Microsoft.AspNet.Mvc
         /// <summary>
         /// Gets <see cref="MediaTypeHeaderValue"/> for the specified format.
         /// </summary>
-        /// <param name="format">format value.</param>
+        /// <param name="format">The format value.</param>
-        /// <returns>The <see cref="MediaTypeHeaderValue"/> for input format</returns>
+        /// <returns>The <see cref="MediaTypeHeaderValue"/> for input format.</returns>
         public MediaTypeHeaderValue GetMediaTypeMappingForFormat([NotNull] string format)
         {
             format = RemovePeriodIfPresent(format);
@@ -46,9 +46,20 @@ namespace Microsoft.AspNet.Mvc
             return value;
         }
 
+        /// <summary>
+        /// Clears the <see cref="MediaTypeHeaderValue"/> mapping for the format.
+        /// </summary>
+        /// <param name="format">The format value.</param>
+        /// <returns><c>true</c> if the format is successfully found and cleared; otherwise, <c>false</c>.</returns>
+        public bool ClearMediaTypeMappingForFormat([NotNull] string format)
+        {
+            format = RemovePeriodIfPresent(format);
+            return _map.Remove(format);
+        }
+
         private void ValidateContentType(MediaTypeHeaderValue contentType)
         {
-            if(contentType.Type == "*" || contentType.SubType == "*")
+            if (contentType.Type == "*" || contentType.SubType == "*")
             {
                 throw new ArgumentException(string.Format(Resources.FormatterMappings_NotValidMediaType, contentType));
             }
@@ -56,10 +67,20 @@ namespace Microsoft.AspNet.Mvc
 
         private string RemovePeriodIfPresent(string format)
         {
+            if (format == "")
+            {
+                throw new ArgumentException(Resources.ArgumentCannotBeNullOrEmpty, "format");
+            }
+
             if (format.StartsWith("."))
             {
-                format = format.Substring(1);
+                if (format == ".")
-            }
+                {
+                    throw new ArgumentException(string.Format(Resources.Format_NotValid, format));
+                }
+
+                format = format.Substring(1);                
+            }            
 
             return format;
         }        

src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs

@@ -562,11 +562,24 @@ namespace Microsoft.AspNet.Mvc.Core
             get { return GetString("Common_ValueNotValidForProperty"); }
         }
 
+        /// <summary>
+        /// The media type "{0}" is not valid. MediaTypes containing wildcards (*) are not allowed in formatter 
+        /// mappings.
+        /// </summary>
         internal static string FormatterMappings_NotValidMediaType
         {
             get { return GetString("FormatterMappings_NotValidMediaType"); }
         }
 
+        /// <summary>
+        /// The format provided is invalid '{0}'. A format must be a non-empty file-extension, optionally prefixed 
+        /// with a '.' character.
+        /// </summary>
+        internal static string Format_NotValid
+        {
+            get { return GetString("Format_NotValid"); }
+        }
+
         /// <summary>
         /// The value '{0}' is invalid.
         /// </summary>

src/Microsoft.AspNet.Mvc.Core/Resources.resx

@@ -452,7 +452,10 @@
     <value>The action '{0}' has ApiExplorer enabled, but is using conventional routing. Only actions which use attribute routing support ApiExplorer.</value>
     </data>
   <data name="FormatterMappings_NotValidMediaType" xml:space="preserve">
-    <value>The media type {0} is not valid. The media type containing "&lt;mediatype&gt;/*" are not supported.</value>
+    <value>The media type "{0}" is not valid. MediaTypes containing wildcards (*) are not allowed in formatter mappings.</value>
+  </data>
+  <data name="Format_NotValid" xml:space="preserve">
+    <value>The format provided is invalid '{0}'. A format must be a non-empty file-extension, optionally prefixed with a '.' character.</value>
   </data>
   <data name="RemoteAttribute_NoUrlFound" xml:space="preserve">
     <value>No URL for remote validation could be found.</value>

src/Microsoft.AspNet.Mvc/MvcOptionsSetup.cs

@@ -6,8 +6,8 @@ using System.Xml.Linq;
 using Microsoft.AspNet.Mvc.ModelBinding;
 using Microsoft.AspNet.Mvc.Razor;
 using Microsoft.Framework.OptionsModel;
-using Newtonsoft.Json.Linq;
 using Microsoft.Net.Http.Headers;
+using Newtonsoft.Json.Linq;
 
 namespace Microsoft.AspNet.Mvc
 {

test/Microsoft.AspNet.Mvc.Core.Test/Filters/FormatFilterTest.cs

@@ -118,7 +118,9 @@ namespace Microsoft.AspNet.Mvc
             var resultExecutingContext = CreateResultExecutingContext(format, place);
             var resourceExecutingContext = CreateResourceExecutingContext(new IFilter[] { }, format, place);
             var options = resultExecutingContext.HttpContext.RequestServices.GetService<IOptions<MvcOptions>>();
-            options.Options.FormatterMappings.SetMediaTypeMappingForFormat(format, MediaTypeHeaderValue.Parse(contentType));
+            options.Options.FormatterMappings.SetMediaTypeMappingForFormat(
+                format, 
+                MediaTypeHeaderValue.Parse(contentType));
             
             var filter = new FormatFilterAttribute();
 
@@ -194,7 +196,28 @@ namespace Microsoft.AspNet.Mvc
             var produces = new ProducesAttribute("application/xml;version=1", new string [] { });
             var context = CreateResourceExecutingContext(new IFilter[] { produces },  "xml", FormatSource.RouteData);
             var options = context.HttpContext.RequestServices.GetService<IOptions<MvcOptions>>();
-            options.Options.FormatterMappings.SetMediaTypeMappingForFormat("xml", MediaTypeHeaderValue.Parse("application/xml"));
+            options.Options.FormatterMappings.SetMediaTypeMappingForFormat(
+                "xml", 
+                MediaTypeHeaderValue.Parse("application/xml"));
+            var filter = new FormatFilterAttribute();
+
+            // Act
+            filter.OnResourceExecuting(context);
+
+            // Assert
+            Assert.Null(context.Result);
+        }
+
+        [Fact]
+        public void FormatFilter_LessSpecificThan_Produces_Wildcard()
+        {
+            // Arrange
+            var produces = new ProducesAttribute("application/*", new string[] { });
+            var context = CreateResourceExecutingContext(new IFilter[] { produces }, "xml", FormatSource.RouteData);
+            var options = context.HttpContext.RequestServices.GetService<IOptions<MvcOptions>>();
+            options.Options.FormatterMappings.SetMediaTypeMappingForFormat(
+                "xml",
+                MediaTypeHeaderValue.Parse("application/xml"));
             var filter = new FormatFilterAttribute();
 
             // Act
@@ -211,7 +234,9 @@ namespace Microsoft.AspNet.Mvc
             var produces = new ProducesAttribute("application/xml", new string[] { });
             var context = CreateResourceExecutingContext(new IFilter[] { produces }, "xml", FormatSource.RouteData);
             var options = context.HttpContext.RequestServices.GetService<IOptions<MvcOptions>>();
-            options.Options.FormatterMappings.SetMediaTypeMappingForFormat("xml", MediaTypeHeaderValue.Parse("application/xml;version=1"));
+            options.Options.FormatterMappings.SetMediaTypeMappingForFormat(
+                "xml", 
+                MediaTypeHeaderValue.Parse("application/xml;version=1"));
             var filter = new FormatFilterAttribute();
 
             // Act

test/Microsoft.AspNet.Mvc.Core.Test/FormatterMappingsTest.cs

@@ -15,7 +15,7 @@ namespace Microsoft.AspNet.Mvc
         [InlineData("json", "application/json", "JSON")]
         [InlineData(".foo", "text/foo", "Foo")]
         [InlineData(".Json", "application/json", "json")]
-        [InlineData("FOo", "text/foo", "FOO")]
+        [InlineData("FOo", "text/foo", "FOO")]        
         public void FormatterMappings_SetFormatMapping_DiffSetGetFormat(string setFormat, string contentType, string getFormat)
         {
             // Arrange
@@ -30,6 +30,38 @@ namespace Microsoft.AspNet.Mvc
             Assert.Equal(mediaType, returnMediaType);
         }
         
+        [Fact]
+        public void FormatterMappings_Invalid_Period()
+        {
+            // Arrange
+            var options = new FormatterMappings();
+            var format = ".";
+            var expected = string.Format(@"The format provided is invalid '{0}'. A format must be a non-empty file-" + 
+                "extension, optionally prefixed with a '.' character.", format);
+
+            // Act and assert
+            var exception = Assert.Throws<ArgumentException>(() => options.SetMediaTypeMappingForFormat(
+                format,
+                MediaTypeHeaderValue.Parse("application/xml")));
+            Assert.Equal(expected, exception.Message);
+        }
+
+        [Fact]
+        public void FormatterMappings_SetFormatMapping_FormatEmpty()
+        {
+            // Arrange
+            var options = new FormatterMappings();
+            var format = "";
+            var expected = "Value cannot be null or empty." + Environment.NewLine + "Parameter name: format";
+
+            // Act and assert
+            var exception = Assert.Throws<ArgumentException>(() => options.SetMediaTypeMappingForFormat(
+                format,
+                MediaTypeHeaderValue.Parse("application/xml")));
+            Assert.Equal(expected, exception.Message);
+        }
+
+
         [Theory]
         [InlineData("application/*")]
         [InlineData("*/json")]
@@ -38,11 +70,45 @@ namespace Microsoft.AspNet.Mvc
         {
             // Arrange
             var options = new FormatterMappings();
-            var expected = string.Format(@"The media type {0} is not valid. The media type containing ""<mediatype>/*"" are not supported.", format);
+            var expected = string.Format(@"The media type ""{0}"" is not valid. MediaTypes containing wildcards (*) " + 
+                "are not allowed in formatter mappings.", format);
 
             // Act and assert
-            var exception = Assert.Throws<ArgumentException>(() => options.SetMediaTypeMappingForFormat("star", MediaTypeHeaderValue.Parse(format)));
+            var exception = Assert.Throws<ArgumentException>(() => options.SetMediaTypeMappingForFormat(
+                "star", 
+                MediaTypeHeaderValue.Parse(format)));
             Assert.Equal(expected, exception.Message);
         }
+
+        [Theory]
+        [InlineData(".xml", true)]
+        [InlineData("json", true)]
+        [InlineData(".foo", true)]
+        [InlineData(".Json", true)]
+        [InlineData("FOo", true)]
+        [InlineData("bar", true)]
+        [InlineData("baz", false)]
+        [InlineData(".baz", false)]
+        [InlineData("BAZ", false)]
+        public void FormatterMappings_ClearFormatMapping(string format, bool expected)
+        {
+            // Arrange
+            var options = new FormatterMappings();
+            var mediaType = MediaTypeHeaderValue.Parse("application/xml");
+            options.SetMediaTypeMappingForFormat("xml", mediaType);
+            mediaType = MediaTypeHeaderValue.Parse("application/json");
+            options.SetMediaTypeMappingForFormat("json", mediaType);
+            mediaType = MediaTypeHeaderValue.Parse("application/foo");
+            options.SetMediaTypeMappingForFormat("foo", mediaType);
+            mediaType = MediaTypeHeaderValue.Parse("application/bar");
+            options.SetMediaTypeMappingForFormat("bar", mediaType);
+
+            // Act 
+            var cleared = options.ClearMediaTypeMappingForFormat(format);
+
+            // Assert
+            Assert.Equal(expected, cleared);
+            Assert.Null(options.GetMediaTypeMappingForFormat(format));
+        }
     }
 }

test/WebSites/ConnegWebSite/Controllers/FormatFilterController.cs

@@ -1,10 +1,9 @@
 // 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 ConnegWebSite;
 using Microsoft.AspNet.Mvc;
 
-namespace ConnegWebsite
+namespace ConnegWebSite
 {
     [Produces("application/FormatFilterController")]
     public class FormatFilterController : Controller

test/WebSites/FormatFilterWebSite/Controllers/ProducesOverrideClass.cs

@@ -11,7 +11,6 @@ namespace FormatFilterWebSite
         [Produces("application/ProducesMethod")]
         public string ReturnClassName()
         {
-            // should be written using the content defined at base class's action.
             return "ProducesOverrideController";
         }
     }        

test/WebSites/FormatFilterWebSite/CustomFormatter.cs

@@ -27,6 +27,8 @@ namespace FormatFilterWebSite
                 var actionReturn = context.Object as Product;
                 if (actionReturn != null)
                 {
+                    var response = context.ActionContext.HttpContext.Response;
+                    context.SelectedContentType = contentType;
                     return true;
                 }
             }
@@ -35,8 +37,7 @@ namespace FormatFilterWebSite
 
         public override async Task WriteResponseBodyAsync(OutputFormatterContext context)
         {
-            var response = context.ActionContext.HttpContext.Response;
+            var response = context.ActionContext.HttpContext.Response;            
-            response.ContentType = ContentType + ";charset=utf-8";
             await response.WriteAsync(context.Object.ToString());
         }
     }