Fix for Content negotiation should fallback to the first formatter that can write the type #1033

- Includes functional and unit tests.

src/Microsoft.AspNet.Mvc.Core/ActionResults/ObjectResult.cs

@@ -86,6 +86,20 @@ namespace Microsoft.AspNet.Mvc
                                                                                 formatterContext,
                                                                                 formatters,
                                                                                 contentTypes);
+
+                    // This would be the case when no formatter could write the type base on the 
+                    // accept headers and the request content type. Fallback on type based match. 
+                    if(selectedFormatter == null)
+                    {
+                        foreach (var formatter in formatters)
+                        {
+                            if (formatter.CanWriteResult(formatterContext, 
+                                                         formatter.SupportedMediaTypes?.FirstOrDefault()))
+                            {
+                                return formatter;
+                            }
+                        }
+                    }
                 }
             }
             else if (ContentTypes.Count == 1)

src/Microsoft.AspNet.Mvc.Core/Formatters/IOutputFormatter.cs

@@ -18,6 +18,19 @@ namespace Microsoft.AspNet.Mvc
     /// </summary>
     public interface IOutputFormatter
     {
+        /// <summary>
+        /// Gets the mutable collection of character encodings supported by
+        /// this <see cref="IOutputFormatter"/>. The encodings are
+        /// used when writing the data.
+        /// </summary>
+        IList<Encoding> SupportedEncodings { get; }
+
+        /// <summary>
+        /// Gets the mutable collection of <see cref="MediaTypeHeaderValue"/> elements supported by
+        /// this <see cref="IOutputFormatter"/>.
+        /// </summary>
+        IList<MediaTypeHeaderValue> SupportedMediaTypes { get; }
+
         /// <summary>
         /// Determines whether this <see cref="IOutputFormatter"/> can serialize
         /// an object of the specified type.

src/Microsoft.AspNet.Mvc.Core/Formatters/NoContentFormatter.cs

@@ -1,6 +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 System;
+using System.Collections.Generic;
+using System.Text;
 using System.Threading.Tasks;
 using Microsoft.AspNet.Mvc.HeaderValueAbstractions;
 
@@ -11,6 +14,10 @@ namespace Microsoft.AspNet.Mvc
     /// </summary>
     public class NoContentFormatter : IOutputFormatter
     {
+        public IList<Encoding> SupportedEncodings { get; private set; }
+
+        public IList<MediaTypeHeaderValue> SupportedMediaTypes { get; private set; }
+
         public bool CanWriteResult(OutputFormatterContext context, MediaTypeHeaderValue contentType)
         {
             // ignore the contentType and just look at the content.

src/Microsoft.AspNet.Mvc.Core/Formatters/OutputFormatter.cs

@@ -17,19 +17,6 @@ namespace Microsoft.AspNet.Mvc
     /// </summary>
     public abstract class OutputFormatter : IOutputFormatter
     {
-        /// <summary>
-        /// Gets the mutable collection of character encodings supported by
-        /// this <see cref="OutputFormatter"/> instance. The encodings are
-        /// used when writing the data.
-        /// </summary>
-        public IList<Encoding> SupportedEncodings { get; private set; }
-
-        /// <summary>
-        /// Gets the mutable collection of <see cref="MediaTypeHeaderValue"/> elements supported by
-        /// this <see cref="OutputFormatter"/> instance.
-        /// </summary>
-        public IList<MediaTypeHeaderValue> SupportedMediaTypes { get; private set; }
-
         /// <summary>
         /// Initializes a new instance of the <see cref="OutputFormatter"/> class.
         /// </summary>
@@ -39,6 +26,12 @@ namespace Microsoft.AspNet.Mvc
             SupportedMediaTypes = new List<MediaTypeHeaderValue>();
         }
 
+        /// <inheritdoc />
+        public IList<Encoding> SupportedEncodings { get; private set; }
+
+        /// <inheritdoc />
+        public IList<MediaTypeHeaderValue> SupportedMediaTypes { get; private set; }
+
         /// <summary>
         /// Determines the best <see cref="Encoding"/> amongst the supported encodings
         /// for reading or writing an HTTP entity body based on the provided <paramref name="contentTypeHeader"/>.

test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/ObjectResultTests.cs

@@ -274,6 +274,52 @@ namespace Microsoft.AspNet.Mvc.Core.Test.ActionResults
             httpResponse.VerifySet(r => r.ContentType = expectedContentType);
         }
 
+        [Theory]
+        [InlineData("", 2)] 
+        [InlineData(null, 2)]
+        [InlineData("application/xml", 3)]
+        [InlineData("application/custom", 3)]
+        [InlineData("application/xml;q=1, application/custom;q=0.8", 4)]
+        public void SelectFormatter_WithNoMatchingAcceptHeadersAndRequestContentType_PicksFormatterBasedOnObjectType
+            (string acceptHeader, int attemptedCountForCanWrite)
+        {
+            // For no accept headers,
+            //can write is called twice once for the request media type and once for the type match pass. 
+            // For each adduaccept header, it is called once. 
+            // Arrange
+            var stream = new MemoryStream();
+            var httpResponse = new Mock<HttpResponse>();
+            httpResponse.SetupProperty<string>(o => o.ContentType);
+            httpResponse.SetupGet(r => r.Body).Returns(stream);
+
+            var actionContext = CreateMockActionContext(httpResponse.Object,
+                                                        requestAcceptHeader: acceptHeader,
+                                                        requestContentType: "application/xml");
+            var input = "testInput";
+            var result = new ObjectResult(input);
+
+            // Set more than one formatters. The test output formatter throws on write.
+            result.Formatters = new List<IOutputFormatter>
+                                    {
+                                        new CannotWriteFormatter(),
+                                        new CountingFormatter(),
+                                    };
+
+            var context = new OutputFormatterContext()
+            {
+                ActionContext = actionContext,
+                Object = input,
+                DeclaredType = typeof(string)
+            };
+
+            // Act
+            var formatter = result.SelectFormatter(context, result.Formatters);
+
+            // Assert
+            var countingFormatter = Assert.IsType<CountingFormatter>(formatter);
+            Assert.Equal(attemptedCountForCanWrite, countingFormatter.GetCanWriteCallCount());
+        }
+
         [Fact]
         public async Task 
             ObjectResult_NoContentTypeSetWithNoAcceptHeadersAndNoRequestContentType_PicksFirstFormatterWhichCanWrite()
@@ -461,21 +507,57 @@ namespace Microsoft.AspNet.Mvc.Core.Test.ActionResults
             return serviceCollection.BuildServiceProvider();
         }
 
+        public class CountingFormatter : OutputFormatter
+        {
+            private int _canWriteCallsCount = 0;
+
+            public CountingFormatter()
+            {
+                SupportedMediaTypes.Add(MediaTypeHeaderValue.Parse("text/plain"));
+                SupportedEncodings.Add(Encoding.GetEncoding("utf-8"));
+            }
+
+            public override bool CanWriteResult(OutputFormatterContext context, MediaTypeHeaderValue contentType)
+            {
+                _canWriteCallsCount++;
+                if (base.CanWriteResult(context, contentType))
+                {
+                    var actionReturnString = context.Object as string;
+                    if (actionReturnString != null)
+                    {
+                        return true;
+                    }
+                }
+
+                return false;
+            }
+
+            public int GetCanWriteCallCount()
+            {
+                return _canWriteCallsCount;
+            }
+
+            public override Task WriteResponseBodyAsync(OutputFormatterContext context)
+            {
+                throw new NotImplementedException();
+            }
+        }
+
         public class CannotWriteFormatter : IOutputFormatter
         {
-            public List<Encoding> SupportedEncodings
+            public IList<Encoding> SupportedEncodings
             {
                 get
                 {
-                    throw new NotImplementedException();
+                    return null;
                 }
             }
 
-            public List<MediaTypeHeaderValue> SupportedMediaTypes
+            public IList<MediaTypeHeaderValue> SupportedMediaTypes
             {
                 get
                 {
-                    throw new NotImplementedException();
+                    return null;
                 }
             }
 

test/Microsoft.AspNet.Mvc.Core.Test/Formatters/OutputFormatterTests.cs

@@ -120,6 +120,24 @@ namespace Microsoft.AspNet.Mvc.Test
                          formatterContext.SelectedContentType.RawValue);
         }
 
+        [Fact]
+        public void CanWriteResult_ForNullContentType_UsesFirstEntryInSupportedContentTypes()
+        {
+            // For no accept headers,
+            //can write is called twice once for the request media type and once for the type match pass. 
+            // For each adduaccept header, it is called once. 
+            // Arrange
+            var context = new OutputFormatterContext();
+            var formatter = new TestOutputFormatter();
+
+            // Act
+            var result = formatter.CanWriteResult(context, null);
+
+            // Assert
+            Assert.True(result);
+            Assert.Equal(formatter.SupportedMediaTypes[0].RawValue, context.SelectedContentType.RawValue);
+        }
+
         private class TestOutputFormatter : OutputFormatter
         {
             public TestOutputFormatter()

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

@@ -113,6 +113,22 @@ namespace Microsoft.AspNet.Mvc
         {
             public Encoding Encoding { get; set; }
 
+            public IList<Encoding> SupportedEncodings
+            {
+                get
+                {
+                    return null;
+                }
+            }
+
+            public IList<MediaTypeHeaderValue> SupportedMediaTypes
+            {
+                get
+                {
+                    return null;
+                }
+            }
+
             public bool CanWriteResult(OutputFormatterContext context, MediaTypeHeaderValue contentType)
             {
                 return true;

test/Microsoft.AspNet.Mvc.Core.Test/OptionDescriptors/OutputFormatterDescriptorTest.cs

@@ -2,6 +2,8 @@
 // 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.Text;
 using System.Threading.Tasks;
 using Microsoft.AspNet.Mvc.HeaderValueAbstractions;
 using Microsoft.AspNet.Mvc.OptionDescriptors;
@@ -55,6 +57,22 @@ namespace Microsoft.AspNet.Mvc.Core
 
         private class TestOutputFormatter : IOutputFormatter
         {
+            public IList<Encoding> SupportedEncodings
+            {
+                get
+                {
+                    return null;
+                }
+            }
+
+            public IList<MediaTypeHeaderValue> SupportedMediaTypes
+            {
+                get
+                {
+                    return null;
+                }
+            }
+
             public bool CanWriteResult(OutputFormatterContext context, MediaTypeHeaderValue contentType)
             {
                 throw new NotImplementedException();

test/Microsoft.AspNet.Mvc.FunctionalTests/ConnegTests.cs

@@ -266,5 +266,67 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
             var body = await response.Content.ReadAsStringAsync();
             Assert.Equal(expectedBody, body);
         }
+
+        [Theory]
+        [InlineData("UseTheFallback_WithDefaultFormatters")]
+        [InlineData("UseTheFallback_UsingCustomFormatters")]
+        public async Task NoMatchOn_RequestContentType_FallsBackOnTypeBasedMatch_MatchFound(string actionName)
+        {
+            // Arrange
+            var server = TestServer.Create(_provider, _app);
+            var client = server.Handler;
+            var expectedContentType = "application/json;charset=utf-8";
+            var expectedBody = "1234";
+            var targetUri = "http://localhost/FallbackOnTypeBasedMatch/" + actionName + "/?input=1234";
+            // Act
+
+            var result = await client.PostAsync(targetUri,
+                                                "1234",
+                                                "application/custom",
+                                                (request) => request.Accept = "application/custom1");
+
+            // Assert
+            Assert.Equal(expectedContentType, result.HttpContext.Response.ContentType);
+            var body = await result.HttpContext.Response.ReadBodyAsStringAsync();
+            Assert.Equal(expectedBody, body);
+        }
+
+        [Theory]
+        [InlineData("OverrideTheFallback_WithDefaultFormatters")]
+        [InlineData("OverrideTheFallback_UsingCustomFormatters")]
+        public async Task NoMatchOn_RequestContentType_SkipTypeMatchByAddingACustomFormatter(string actionName)
+        {
+            // Arrange
+            var server = TestServer.Create(_provider, _app);
+            var client = server.Handler;
+            var targetUri = "http://localhost/FallbackOnTypeBasedMatch/" + actionName + "/?input=1234";
+
+            // Act
+            var result = await client.PostAsync(targetUri,
+                                                "1234",
+                                                "application/custom",
+                                                (request) => request.Accept = "application/custom1");
+
+            // Assert
+            Assert.Equal(406, result.HttpContext.Response.StatusCode);
+        }
+
+        [Fact]
+        public async Task NoMatchOn_RequestContentType_FallsBackOnTypeBasedMatch_NoMatchFound_Returns406()
+        {
+            // Arrange
+            var server = TestServer.Create(_provider, _app);
+            var client = server.Handler;
+            var targetUri = "http://localhost/FallbackOnTypeBasedMatch/FallbackGivesNoMatch/?input=1234";
+            
+            // Act
+            var result = await client.PostAsync(targetUri,
+                                                "1234",
+                                                "application/custom",
+                                                (request) => request.Accept = "application/custom1");
+
+            // Assert
+            Assert.Equal(406, result.HttpContext.Response.StatusCode);
+        }
     }
 }

test/WebSites/ConnegWebSite/Controllers/FallbackOnTypeBasedMatchController.cs

@@ -0,0 +1,101 @@
+// 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.Collections.Generic;
+using System.IO;
+using System.Text;
+using System.Threading.Tasks;
+using Microsoft.AspNet.Http;
+using Microsoft.AspNet.Mvc;
+using Microsoft.AspNet.Mvc.HeaderValueAbstractions;
+using Microsoft.Framework.DependencyInjection;
+namespace ConnegWebsite
+{
+    public class FallbackOnTypeBasedMatchController : Controller
+    {
+        public int UseTheFallback_WithDefaultFormatters(int input)
+        {
+            return input;
+        }
+
+        public IActionResult UseTheFallback_UsingCustomFormatters(int input)
+        {
+            var objectResult = new ObjectResult(input);
+
+            // Request content type is application/custom.
+            // PlainTextFormatter cannot write because it does not support the type. 
+            // JsonOutputFormatter cannot write in the first attempt because it does not support the 
+            // request content type.
+            objectResult.Formatters.Add(new PlainTextFormatter());
+            objectResult.Formatters.Add(new JsonOutputFormatter(JsonOutputFormatter.CreateDefaultSettings(), false));
+
+            return objectResult;
+        }
+
+        public IActionResult FallbackGivesNoMatch(int input)
+        {
+            var objectResult = new ObjectResult(input);
+
+            // Request content type is application/custom.
+            // PlainTextFormatter cannot write because it does not support the type. 
+            objectResult.Formatters.Add(new PlainTextFormatter());
+
+            return objectResult;
+        }
+
+        public IActionResult OverrideTheFallback_UsingCustomFormatters(int input)
+        {
+            var objectResult = new ObjectResult(input);
+            objectResult.Formatters.Add(new StopIfNoMatchOutputFormatter());
+            objectResult.Formatters.Add(new PlainTextFormatter());
+            objectResult.Formatters.Add(new JsonOutputFormatter(JsonOutputFormatter.CreateDefaultSettings(), false));
+            return objectResult;
+        }
+
+        public IActionResult OverrideTheFallback_WithDefaultFormatters(int input)
+        {
+            var objectResult = new ObjectResult(input);
+            var formattersProvider = ActionContext.HttpContext.RequestServices.GetService<IOutputFormattersProvider>();
+            objectResult.Formatters.Add(new StopIfNoMatchOutputFormatter());
+            foreach (var formatter in formattersProvider.OutputFormatters)
+            {
+                objectResult.Formatters.Add(formatter);
+            }
+
+            return objectResult;
+        }
+
+        public class StopIfNoMatchOutputFormatter : IOutputFormatter
+        {
+            public IList<Encoding> SupportedEncodings
+            {
+                get
+                {
+                    return null;
+                }
+            }
+
+            public IList<MediaTypeHeaderValue> SupportedMediaTypes
+            {
+                get
+                {
+                    return null;
+                }
+            }
+
+            // Select if no Registered content type.
+            public bool CanWriteResult(OutputFormatterContext context, MediaTypeHeaderValue contentType)
+            {
+                return contentType == null;
+            }
+
+            public Task WriteAsync(OutputFormatterContext context)
+            {
+                var response = context.ActionContext.HttpContext.Response;
+                response.StatusCode = 406;
+                return Task.FromResult<bool>(true);
+            }
+        }
+    }
+}