Fix for codeplex-1120 - Move CreateSerializer out of the base class

This is a small refactor as a precursor for api-explorer work.

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

@@ -23,10 +23,25 @@ namespace Microsoft.AspNet.Mvc
         /// for the <paramref name="declaredType"/> and <paramref name="contentType"/>.
         /// </summary>
         /// <param name="declaredType">The declared type for which the supported content types are desired.</param>
-        /// <param name="instanceType">The runtime type for which the supported content types are desired.</param>
+        /// <param name="runtimeType">The runtime type for which the supported content types are desired.</param>
-        /// <param name="contentType">Content type for which the supported content types are desired.</param>
+        /// <param name="contentType">
+        /// The content type for which the supported content types are desired, or <c>null</c> if any content 
+        /// type can be used.
+        /// </param>
         /// <returns>Content types which are supported by this formatter.</returns>
-        IReadOnlyList<MediaTypeHeaderValue> GetSupportedContentTypes(Type declaredType, Type instanceType, MediaTypeHeaderValue contentType);
+        /// <remarks>
+        /// If the value of <paramref name="contentType"/> is <c>null</c>, then the formatter should return a list
+        /// of all content types that it can produce for the given data type. This may occur during content 
+        /// negotiation when the HTTP Accept header is not specified, or when no match for the value in the Accept
+        /// header can be found.
+        /// 
+        /// If the value of <paramref name="contentType"/> is not <c>null</c>, then the formatter should return
+        /// a list of all content types that it can produce which match the given data type and content type.
+        /// </remarks>
+        IReadOnlyList<MediaTypeHeaderValue> GetSupportedContentTypes(
+            Type declaredType, 
+            Type runtimeType, 
+            MediaTypeHeaderValue contentType);
 
         /// <summary>
         /// Determines whether this <see cref="IOutputFormatter"/> can serialize

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

@@ -5,7 +5,6 @@ using System;
 using System.Collections.Generic;
 using System.Linq;
 using System.Text;
-using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.AspNet.Mvc.Core;
 using Microsoft.AspNet.Mvc.HeaderValueAbstractions;
@@ -17,13 +16,16 @@ namespace Microsoft.AspNet.Mvc
     /// </summary>
     public abstract class OutputFormatter : IOutputFormatter
     {
+        // using a field so we can return it as both IList and IReadOnlyList
+        private readonly List<MediaTypeHeaderValue> _supportedMediaTypes;
+
         /// <summary>
         /// Initializes a new instance of the <see cref="OutputFormatter"/> class.
         /// </summary>
         protected OutputFormatter()
         {
             SupportedEncodings = new List<Encoding>();
-            SupportedMediaTypes = new List<MediaTypeHeaderValue>();
+            _supportedMediaTypes = new List<MediaTypeHeaderValue>();
         }
 
         /// <summary>
@@ -37,23 +39,57 @@ namespace Microsoft.AspNet.Mvc
         /// Gets the mutable collection of <see cref="MediaTypeHeaderValue"/> elements supported by
         /// this <see cref="OutputFormatter"/>.
         /// </summary>
-        public IList<MediaTypeHeaderValue> SupportedMediaTypes { get; private set; }
+        public IList<MediaTypeHeaderValue> SupportedMediaTypes
+        {
+            get { return _supportedMediaTypes; }
+        }
+
+        /// <summary>
+        /// Returns a value indicating whether or not the given type can be written by this serializer.
+        /// </summary>
+        /// <param name="declaredType">The declared type.</param>
+        /// <param name="runtimeType">The runtime type.</param>
+        /// <returns><c>true</c> if the type can be written, otherwise <c>false</c>.</returns>
+        protected virtual bool CanWriteType(Type declaredType, Type runtimeType)
+        {
+            return true;
+        }
 
         /// <inheritdoc />
-        public virtual IReadOnlyList<MediaTypeHeaderValue> GetSupportedContentTypes(Type declaredType, Type runtimeType, MediaTypeHeaderValue contentType)
+        public virtual IReadOnlyList<MediaTypeHeaderValue> GetSupportedContentTypes(
+            Type declaredType,
+            Type runtimeType,
+            MediaTypeHeaderValue contentType)
         {
-            var mediaTypes = new List<MediaTypeHeaderValue>();
+            if (!CanWriteType(declaredType, runtimeType))
+            {
+                return null;
+            }
 
             if (contentType == null)
             {
-                mediaTypes.AddRange(SupportedMediaTypes);
+                // If contentType is null, then any type we support is valid.
+                return _supportedMediaTypes.Count > 0 ? _supportedMediaTypes : null;
             }
             else
             {
-                mediaTypes.Add(SupportedMediaTypes.FirstOrDefault(mt => mt.IsSubsetOf(contentType)));
+                List<MediaTypeHeaderValue> mediaTypes = null;
-            }
 
-            return mediaTypes;
+                foreach (var mediaType in _supportedMediaTypes)
+                {
+                    if (mediaType.IsSubsetOf(contentType))
+                    {
+                        if (mediaTypes == null)
+                        {
+                            mediaTypes = new List<MediaTypeHeaderValue>();
+                        }
+
+                        mediaTypes.Add(mediaType);
+                    }
+                }
+
+                return mediaTypes;
+            }
         }
 
         /// <summary>
@@ -63,7 +99,7 @@ namespace Microsoft.AspNet.Mvc
         /// <param name="context">The formatter context associated with the call.
         /// </param>
         /// <returns>The <see cref="Encoding"/> to use when reading the request or writing the response.</returns>
-        public virtual Encoding SelectCharacterEncoding(OutputFormatterContext context)
+        public virtual Encoding SelectCharacterEncoding([NotNull] OutputFormatterContext context)
         {
             var request = context.ActionContext.HttpContext.Request;
             var encoding = MatchAcceptCharacterEncoding(request.AcceptCharset);
@@ -85,28 +121,15 @@ namespace Microsoft.AspNet.Mvc
             return encoding;
         }
 
-        /// <summary>
+        /// <inheritdoc />
-        /// Gets the type of the object to be serialized.
+        public virtual bool CanWriteResult([NotNull] OutputFormatterContext context, MediaTypeHeaderValue contentType)
-        /// </summary>
-        /// <param name="context">The context which contains the object to be serialized.</param>
-        /// <returns>The type of the object to be serialized.</returns>
-        public virtual Type GetObjectType([NotNull] OutputFormatterContext context)
         {
-            if (context.DeclaredType == null ||
+            var runtimeType = context.Object == null ? null : context.Object.GetType();
-                context.DeclaredType == typeof(object))
+            if (!CanWriteType(context.DeclaredType, runtimeType))
             {
-                if (context.Object != null)
+                return false;
-                {
-                    return context.Object.GetType();
-                }
             }
 
-            return context.DeclaredType;
-        }
-
-        /// <inheritdoc />
-        public virtual bool CanWriteResult(OutputFormatterContext context, MediaTypeHeaderValue contentType)
-        {
             MediaTypeHeaderValue mediaType = null;
             if (contentType == null)
             {
@@ -132,7 +155,7 @@ namespace Microsoft.AspNet.Mvc
         }
 
         /// <inheritdoc />
-        public async Task WriteAsync(OutputFormatterContext context)
+        public async Task WriteAsync([NotNull] OutputFormatterContext context)
         {
             WriteResponseContentHeaders(context);
             await WriteResponseBodyAsync(context);
@@ -142,7 +165,7 @@ namespace Microsoft.AspNet.Mvc
         /// Sets the content-type headers with charset value to the HttpResponse.
         /// </summary>
         /// <param name="context">The formatter context associated with the call.</param>
-        public virtual void WriteResponseContentHeaders(OutputFormatterContext context)
+        public virtual void WriteResponseContentHeaders([NotNull] OutputFormatterContext context)
         {
             var selectedMediaType = context.SelectedContentType;
 
@@ -175,7 +198,7 @@ namespace Microsoft.AspNet.Mvc
         /// </summary>
         /// <param name="context">The formatter context associated with the call.</param>
         /// <returns>A task which can write the response body.</returns>
-        public abstract Task WriteResponseBodyAsync(OutputFormatterContext context);
+        public abstract Task WriteResponseBodyAsync([NotNull] OutputFormatterContext context);
 
         private Encoding MatchAcceptCharacterEncoding(string acceptCharsetHeader)
         {

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

@@ -32,14 +32,19 @@ namespace Microsoft.AspNet.Mvc
         {
         }
 
+        /// <inheritdoc />
+        protected override bool CanWriteType(Type declaredType, Type runtimeType)
+        {
+            return CreateSerializer(GetSerializableType(declaredType, runtimeType)) != null;
+        }
+
         /// <summary>
         /// Create a new instance of <see cref="DataContractSerializer"/> for the given object type.
         /// </summary>
         /// <param name="type">The type of object for which the serializer should be created.</param>
         /// <returns>A new instance of <see cref="DataContractSerializer"/></returns>
-        public override object CreateSerializer([NotNull] Type type)
+        protected virtual DataContractSerializer CreateSerializer([NotNull] Type type)
         {
-            DataContractSerializer serializer = null;
             try
             {
 #if ASPNET50
@@ -47,15 +52,14 @@ namespace Microsoft.AspNet.Mvc
                 FormattingUtilities.XsdDataContractExporter.GetRootElementName(type);
 #endif
                 // If the serializer does not support this type it will throw an exception.
-                serializer = new DataContractSerializer(type);
+                return new DataContractSerializer(type);
             }
             catch (Exception)
             {
                 // We do not surface the caught exception because if CanWriteResult returns
                 // false, then this Formatter is not picked up at all.
+                return null;
             }
-
-            return serializer;
         }
 
         /// <inheritdoc />
@@ -69,7 +73,10 @@ namespace Microsoft.AspNet.Mvc
             using (var outputStream = new DelegatingStream(innerStream))
             using (var xmlWriter = CreateXmlWriter(outputStream, tempWriterSettings))
             {
-                var dataContractSerializer = (DataContractSerializer)CreateSerializer(GetObjectType(context));
+                var runtimeType = context.Object == null ? null : context.Object.GetType();
+
+                var type = GetSerializableType(context.DeclaredType, runtimeType);
+                var dataContractSerializer = CreateSerializer(type);
                 dataContractSerializer.WriteObject(xmlWriter, context.Object);
             }
 

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

@@ -30,24 +30,23 @@ namespace Microsoft.AspNet.Mvc
         public XmlWriterSettings WriterSettings { get; private set; }
 
         /// <summary>
-        /// Returns a serializer to serialzie the particualr type.
+        /// Gets the type of the object to be serialized.
         /// </summary>
-        /// <param name="type">The type which needs to be serialized.</param>
+        /// <param name="declaredType">The declared type.</param>
-        /// <returns>The serializer object.</returns>
+        /// <param name="runtimeType">The runtime type.</param>
-        public abstract object CreateSerializer(Type type);
+        /// <returns>The type of the object to be serialized.</returns>
-
+        protected virtual Type GetSerializableType(Type declaredType, Type runtimeType)
-        /// <inheritdoc />
-        public override bool CanWriteResult([NotNull] OutputFormatterContext context, MediaTypeHeaderValue contentType)
         {
-            if (base.CanWriteResult(context, contentType))
+            if (declaredType == null ||
+                declaredType == typeof(object))
             {
-                if (CreateSerializer(GetObjectType(context)) != null)
+                if (runtimeType != null)
                 {
-                    return true;
+                    return runtimeType;
                 }
             }
 
-            return false;
+            return declaredType;
         }
 
         /// <summary>

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

@@ -32,26 +32,30 @@ namespace Microsoft.AspNet.Mvc
         {
         }
 
+        /// <inheritdoc />
+        protected override bool CanWriteType(Type declaredType, Type runtimeType)
+        {
+            return CreateSerializer(GetSerializableType(declaredType, runtimeType)) != null;
+        }
+
         /// <summary>
         /// Create a new instance of <see cref="XmlSerializer"/> for the given object type.
         /// </summary>
         /// <param name="type">The type of object for which the serializer should be created.</param>
         /// <returns>A new instance of <see cref="XmlSerializer"/></returns>
-        public override object CreateSerializer([NotNull] Type type)
+        protected virtual XmlSerializer CreateSerializer([NotNull] Type type)
         {
-            XmlSerializer serializer = null;
             try
             {
                 // If the serializer does not support this type it will throw an exception.
-                serializer = new XmlSerializer(type);
+                return new XmlSerializer(type);
             }
             catch (Exception)
             {
                 // We do not surface the caught exception because if CanWriteResult returns
                 // false, then this Formatter is not picked up at all.
+                return null;
             }
-
-            return serializer;
         }
 
         /// <inheritdoc />
@@ -67,7 +71,10 @@ namespace Microsoft.AspNet.Mvc
             using (var outputStream = new DelegatingStream(innerStream))
             using (var xmlWriter = CreateXmlWriter(outputStream, tempWriterSettings))
             {
-                var xmlSerializer = (XmlSerializer)CreateSerializer(GetObjectType(context));
+                var runtimeType = context.Object == null ? null : context.Object.GetType();
+
+                var type = GetSerializableType(context.DeclaredType, runtimeType);
+                var xmlSerializer = CreateSerializer(type);
                 xmlSerializer.Serialize(xmlWriter, context.Object);
             }
 

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

@@ -135,6 +135,141 @@ namespace Microsoft.AspNet.Mvc.Test
             Assert.Equal(formatter.SupportedMediaTypes[0].RawValue, context.SelectedContentType.RawValue);
         }
 
+        [Fact]
+        public void GetSupportedContentTypes_ReturnsNull_ForUnsupportedType()
+        {
+            // Arrange
+            var formatter = new TypeSpecificFormatter();
+
+            formatter.SupportedMediaTypes.Add(MediaTypeHeaderValue.Parse("application/json"));
+
+            formatter.SupportedTypes.Add(typeof(int));
+
+            // Act
+            var contentTypes = formatter.GetSupportedContentTypes(
+                declaredType: typeof(string), 
+                runtimeType: typeof(string), 
+                contentType: null);
+
+            // Assert
+            Assert.Null(contentTypes);
+        }
+
+        [Fact]
+        public void CanWrite_ReturnsFalse_ForUnsupportedType()
+        {
+            // Arrange
+            var context = new OutputFormatterContext();
+            context.DeclaredType = typeof(string);
+            context.Object = "Hello, world!";
+
+            var formatter = new TypeSpecificFormatter();
+
+            formatter.SupportedMediaTypes.Add(MediaTypeHeaderValue.Parse("application/json"));
+
+            formatter.SupportedTypes.Add(typeof(int));
+
+            // Act
+            var result = formatter.CanWriteResult(context, formatter.SupportedMediaTypes[0]);
+
+            // Assert
+            Assert.False(result);
+        }
+
+        [Fact]
+        public void GetSupportedContentTypes_ReturnsAllContentTypes_WithContentTypeNull()
+        {
+            // Arrange
+            var formatter = new TestOutputFormatter();
+
+            formatter.SupportedMediaTypes.Clear();
+            formatter.SupportedMediaTypes.Add(MediaTypeHeaderValue.Parse("application/json"));
+            formatter.SupportedMediaTypes.Add(MediaTypeHeaderValue.Parse("application/xml"));
+
+            // Act
+            var contentTypes = formatter.GetSupportedContentTypes(typeof(int), typeof(int), contentType: null);
+
+            // Assert
+            Assert.Equal(2, contentTypes.Count);
+            Assert.Single(contentTypes, ct => ct.RawValue == "application/json");
+            Assert.Single(contentTypes, ct => ct.RawValue == "application/xml");
+        }
+
+        [Fact]
+        public void GetSupportedContentTypes_ReturnsMatchingContentTypes_WithContentType()
+        {
+            // Arrange
+            var formatter = new TestOutputFormatter();
+
+            formatter.SupportedMediaTypes.Clear();
+            formatter.SupportedMediaTypes.Add(MediaTypeHeaderValue.Parse("application/json"));
+            formatter.SupportedMediaTypes.Add(MediaTypeHeaderValue.Parse("text/xml"));
+
+            // Act
+            var contentTypes = formatter.GetSupportedContentTypes(
+                typeof(int), 
+                typeof(int), 
+                contentType: MediaTypeHeaderValue.Parse("application/*"));
+
+            // Assert
+            var contentType = Assert.Single(contentTypes);
+            Assert.Equal("application/json", contentType.RawValue);
+        }
+
+        [Fact]
+        public void GetSupportedContentTypes_ReturnsMatchingContentTypes_NoMatches()
+        {
+            // Arrange
+            var formatter = new TestOutputFormatter();
+
+            formatter.SupportedMediaTypes.Clear();
+            formatter.SupportedMediaTypes.Add(MediaTypeHeaderValue.Parse("application/json"));
+            formatter.SupportedMediaTypes.Add(MediaTypeHeaderValue.Parse("text/xml"));
+
+            // Act
+            var contentTypes = formatter.GetSupportedContentTypes(
+                typeof(int),
+                typeof(int),
+                contentType: MediaTypeHeaderValue.Parse("application/xml"));
+
+            // Assert
+            Assert.Null(contentTypes);
+        }
+
+        [Fact]
+        public void GetSupportedContentTypes_ReturnsAllContentTypes_ReturnsNullWithNoSupportedContentTypes()
+        {
+            // Arrange
+            var formatter = new TestOutputFormatter();
+
+            // Intentionally empty
+            formatter.SupportedMediaTypes.Clear();
+
+            // Act
+            var contentTypes = formatter.GetSupportedContentTypes(
+                typeof(int),
+                typeof(int),
+                contentType: null);
+
+            // Assert
+            Assert.Null(contentTypes);
+        }
+
+        private class TypeSpecificFormatter : OutputFormatter
+        {
+            public List<Type> SupportedTypes { get; } = new List<Type>();
+
+            protected override bool CanWriteType(Type declaredType, Type runtimeType)
+            {
+                return SupportedTypes.Contains(declaredType ?? runtimeType);
+            }
+
+            public override Task WriteResponseBodyAsync(OutputFormatterContext context)
+            {
+                throw new NotImplementedException();
+            }
+        }
+
         private class TestOutputFormatter : OutputFormatter
         {
             public TestOutputFormatter()