From 529b17ea7037f19fe05d0e393d0cabecd29b41c4 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Tue, 16 Sep 2014 12:49:35 -0700 Subject: [PATCH] Fix for codeplex-1120 - Move CreateSerializer out of the base class This is a small refactor as a precursor for api-explorer work. --- .../Formatters/IOutputFormatter.cs | 21 ++- .../Formatters/OutputFormatter.cs | 85 +++++++---- ...mlDataContractSerializerOutputFormatter.cs | 19 ++- .../Formatters/XmlOutputFormatter.cs | 21 ++- .../XmlSerializerOutputFormatter.cs | 19 ++- .../Formatters/OutputFormatterTests.cs | 135 ++++++++++++++++++ 6 files changed, 243 insertions(+), 57 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/Formatters/IOutputFormatter.cs b/src/Microsoft.AspNet.Mvc.Core/Formatters/IOutputFormatter.cs index 9d04e598b7..b6a8892d43 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Formatters/IOutputFormatter.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Formatters/IOutputFormatter.cs @@ -23,10 +23,25 @@ namespace Microsoft.AspNet.Mvc /// for the and . /// /// The declared type for which the supported content types are desired. - /// The runtime type for which the supported content types are desired. - /// Content type for which the supported content types are desired. + /// The runtime type for which the supported content types are desired. + /// + /// The content type for which the supported content types are desired, or null if any content + /// type can be used. + /// /// Content types which are supported by this formatter. - IReadOnlyList GetSupportedContentTypes(Type declaredType, Type instanceType, MediaTypeHeaderValue contentType); + /// + /// If the value of is null, 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 is not null, then the formatter should return + /// a list of all content types that it can produce which match the given data type and content type. + /// + IReadOnlyList GetSupportedContentTypes( + Type declaredType, + Type runtimeType, + MediaTypeHeaderValue contentType); /// /// Determines whether this can serialize diff --git a/src/Microsoft.AspNet.Mvc.Core/Formatters/OutputFormatter.cs b/src/Microsoft.AspNet.Mvc.Core/Formatters/OutputFormatter.cs index 4237214a2d..213a193738 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Formatters/OutputFormatter.cs +++ b/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 /// public abstract class OutputFormatter : IOutputFormatter { + // using a field so we can return it as both IList and IReadOnlyList + private readonly List _supportedMediaTypes; + /// /// Initializes a new instance of the class. /// protected OutputFormatter() { SupportedEncodings = new List(); - SupportedMediaTypes = new List(); + _supportedMediaTypes = new List(); } /// @@ -37,23 +39,57 @@ namespace Microsoft.AspNet.Mvc /// Gets the mutable collection of elements supported by /// this . /// - public IList SupportedMediaTypes { get; private set; } + public IList SupportedMediaTypes + { + get { return _supportedMediaTypes; } + } + + /// + /// Returns a value indicating whether or not the given type can be written by this serializer. + /// + /// The declared type. + /// The runtime type. + /// true if the type can be written, otherwise false. + protected virtual bool CanWriteType(Type declaredType, Type runtimeType) + { + return true; + } /// - public virtual IReadOnlyList GetSupportedContentTypes(Type declaredType, Type runtimeType, MediaTypeHeaderValue contentType) + public virtual IReadOnlyList GetSupportedContentTypes( + Type declaredType, + Type runtimeType, + MediaTypeHeaderValue contentType) { - var mediaTypes = new List(); + 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 mediaTypes = null; - return mediaTypes; + foreach (var mediaType in _supportedMediaTypes) + { + if (mediaType.IsSubsetOf(contentType)) + { + if (mediaTypes == null) + { + mediaTypes = new List(); + } + + mediaTypes.Add(mediaType); + } + } + + return mediaTypes; + } } /// @@ -63,7 +99,7 @@ namespace Microsoft.AspNet.Mvc /// The formatter context associated with the call. /// /// The to use when reading the request or writing the response. - 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; } - /// - /// Gets the type of the object to be serialized. - /// - /// The context which contains the object to be serialized. - /// The type of the object to be serialized. - public virtual Type GetObjectType([NotNull] OutputFormatterContext context) + /// + public virtual bool CanWriteResult([NotNull] OutputFormatterContext context, MediaTypeHeaderValue contentType) { - if (context.DeclaredType == null || - context.DeclaredType == typeof(object)) + var runtimeType = context.Object == null ? null : context.Object.GetType(); + if (!CanWriteType(context.DeclaredType, runtimeType)) { - if (context.Object != null) - { - return context.Object.GetType(); - } + return false; } - return context.DeclaredType; - } - - /// - public virtual bool CanWriteResult(OutputFormatterContext context, MediaTypeHeaderValue contentType) - { MediaTypeHeaderValue mediaType = null; if (contentType == null) { @@ -132,7 +155,7 @@ namespace Microsoft.AspNet.Mvc } /// - 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. /// /// The formatter context associated with the call. - 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 /// /// The formatter context associated with the call. /// A task which can write the response body. - public abstract Task WriteResponseBodyAsync(OutputFormatterContext context); + public abstract Task WriteResponseBodyAsync([NotNull] OutputFormatterContext context); private Encoding MatchAcceptCharacterEncoding(string acceptCharsetHeader) { diff --git a/src/Microsoft.AspNet.Mvc.Core/Formatters/XmlDataContractSerializerOutputFormatter.cs b/src/Microsoft.AspNet.Mvc.Core/Formatters/XmlDataContractSerializerOutputFormatter.cs index 786e83ad36..b7fcbbdd74 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Formatters/XmlDataContractSerializerOutputFormatter.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Formatters/XmlDataContractSerializerOutputFormatter.cs @@ -32,14 +32,19 @@ namespace Microsoft.AspNet.Mvc { } + /// + protected override bool CanWriteType(Type declaredType, Type runtimeType) + { + return CreateSerializer(GetSerializableType(declaredType, runtimeType)) != null; + } + /// /// Create a new instance of for the given object type. /// /// The type of object for which the serializer should be created. /// A new instance of - 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; } /// @@ -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); } diff --git a/src/Microsoft.AspNet.Mvc.Core/Formatters/XmlOutputFormatter.cs b/src/Microsoft.AspNet.Mvc.Core/Formatters/XmlOutputFormatter.cs index f9135b3208..86eaae8e3e 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Formatters/XmlOutputFormatter.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Formatters/XmlOutputFormatter.cs @@ -30,24 +30,23 @@ namespace Microsoft.AspNet.Mvc public XmlWriterSettings WriterSettings { get; private set; } /// - /// Returns a serializer to serialzie the particualr type. + /// Gets the type of the object to be serialized. /// - /// The type which needs to be serialized. - /// The serializer object. - public abstract object CreateSerializer(Type type); - - /// - public override bool CanWriteResult([NotNull] OutputFormatterContext context, MediaTypeHeaderValue contentType) + /// The declared type. + /// The runtime type. + /// The type of the object to be serialized. + protected virtual Type GetSerializableType(Type declaredType, Type runtimeType) { - 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; } /// diff --git a/src/Microsoft.AspNet.Mvc.Core/Formatters/XmlSerializerOutputFormatter.cs b/src/Microsoft.AspNet.Mvc.Core/Formatters/XmlSerializerOutputFormatter.cs index 99bae29710..fc48093881 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Formatters/XmlSerializerOutputFormatter.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Formatters/XmlSerializerOutputFormatter.cs @@ -32,26 +32,30 @@ namespace Microsoft.AspNet.Mvc { } + /// + protected override bool CanWriteType(Type declaredType, Type runtimeType) + { + return CreateSerializer(GetSerializableType(declaredType, runtimeType)) != null; + } + /// /// Create a new instance of for the given object type. /// /// The type of object for which the serializer should be created. /// A new instance of - 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; } /// @@ -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); } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/OutputFormatterTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/OutputFormatterTests.cs index 70479e446c..5fe71f035f 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/OutputFormatterTests.cs +++ b/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 SupportedTypes { get; } = new List(); + + 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()