diff --git a/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/Internal/MvcXmlDataContractSerializerMvcOptionsSetup.cs b/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/Internal/MvcXmlDataContractSerializerMvcOptionsSetup.cs index ae71646600..dbe7a8ca82 100644 --- a/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/Internal/MvcXmlDataContractSerializerMvcOptionsSetup.cs +++ b/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/Internal/MvcXmlDataContractSerializerMvcOptionsSetup.cs @@ -1,8 +1,10 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; +using System; using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal @@ -13,6 +15,22 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal /// public class MvcXmlDataContractSerializerMvcOptionsSetup : IConfigureOptions { + private readonly ILoggerFactory _loggerFactory; + + /// + /// Initializes a new instance of . + /// + /// The . + public MvcXmlDataContractSerializerMvcOptionsSetup(ILoggerFactory loggerFactory) + { + if (loggerFactory == null) + { + throw new ArgumentNullException(nameof(loggerFactory)); + } + + _loggerFactory = loggerFactory; + } + /// /// Adds the data contract serializer formatters to . /// @@ -21,7 +39,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal { options.ModelMetadataDetailsProviders.Add(new DataMemberRequiredBindingMetadataProvider()); - options.OutputFormatters.Add(new XmlDataContractSerializerOutputFormatter()); + options.OutputFormatters.Add(new XmlDataContractSerializerOutputFormatter(_loggerFactory)); options.InputFormatters.Add(new XmlDataContractSerializerInputFormatter(options)); options.ModelMetadataDetailsProviders.Add(new SuppressChildValidationMetadataProvider("System.Xml.Linq.XObject")); diff --git a/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/Internal/MvcXmlSerializerMvcOptionsSetup.cs b/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/Internal/MvcXmlSerializerMvcOptionsSetup.cs index b072db2b12..e2b75d7f89 100644 --- a/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/Internal/MvcXmlSerializerMvcOptionsSetup.cs +++ b/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/Internal/MvcXmlSerializerMvcOptionsSetup.cs @@ -1,6 +1,8 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal @@ -11,13 +13,29 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal /// public class MvcXmlSerializerMvcOptionsSetup : IConfigureOptions { + private readonly ILoggerFactory _loggerFactory; + + /// + /// Initializes a new instance of . + /// + /// The . + public MvcXmlSerializerMvcOptionsSetup(ILoggerFactory loggerFactory) + { + if (loggerFactory == null) + { + throw new ArgumentNullException(nameof(loggerFactory)); + } + + _loggerFactory = loggerFactory; + } + /// /// Adds the XML serializer formatters to . /// /// The . public void Configure(MvcOptions options) { - options.OutputFormatters.Add(new XmlSerializerOutputFormatter()); + options.OutputFormatters.Add(new XmlSerializerOutputFormatter(_loggerFactory)); options.InputFormatters.Add(new XmlSerializerInputFormatter(options)); } } diff --git a/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/LoggerExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/LoggerExtensions.cs new file mode 100644 index 0000000000..dfb00627d4 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/LoggerExtensions.cs @@ -0,0 +1,37 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal +{ + public static class LoggerExtensions + { + private static readonly Action _failedToCreateXmlSerializer; + private static readonly Action _failedToCreateDataContractSerializer; + + static LoggerExtensions() + { + _failedToCreateXmlSerializer = LoggerMessage.Define( + LogLevel.Warning, + 1, + "An error occurred while trying to create an XmlSerializer for the type '{Type}'."); + + _failedToCreateDataContractSerializer = LoggerMessage.Define( + LogLevel.Warning, + 2, + "An error occurred while trying to create a DataContractSerializer for the type '{Type}'."); + } + + public static void FailedToCreateXmlSerializer(this ILogger logger, string typeName, Exception exception) + { + _failedToCreateXmlSerializer(logger, typeName, exception); + } + + public static void FailedToCreateDataContractSerializer(this ILogger logger, string typeName, Exception exception) + { + _failedToCreateDataContractSerializer(logger, typeName, exception); + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/XmlDataContractSerializerOutputFormatter.cs b/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/XmlDataContractSerializerOutputFormatter.cs index 31d22e625c..5ce5dabc82 100644 --- a/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/XmlDataContractSerializerOutputFormatter.cs +++ b/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/XmlDataContractSerializerOutputFormatter.cs @@ -11,6 +11,7 @@ using System.Threading.Tasks; using System.Xml; using Microsoft.AspNetCore.Mvc.Formatters.Xml; using Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal; +using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Mvc.Formatters { @@ -21,22 +22,43 @@ namespace Microsoft.AspNetCore.Mvc.Formatters public class XmlDataContractSerializerOutputFormatter : TextOutputFormatter { private readonly ConcurrentDictionary _serializerCache = new ConcurrentDictionary(); + private readonly ILogger _logger; private DataContractSerializerSettings _serializerSettings; /// /// Initializes a new instance of - /// with default XmlWriterSettings + /// with default . /// - public XmlDataContractSerializerOutputFormatter() : - this(FormattingUtilities.GetDefaultXmlWriterSettings()) + public XmlDataContractSerializerOutputFormatter() + : this(FormattingUtilities.GetDefaultXmlWriterSettings()) { } /// /// Initializes a new instance of + /// with default . + /// + /// The . + public XmlDataContractSerializerOutputFormatter(ILoggerFactory loggerFactory) + : this(FormattingUtilities.GetDefaultXmlWriterSettings(), loggerFactory) + { + } + + /// + /// Initializes a new instance of . /// /// The settings to be used by the . public XmlDataContractSerializerOutputFormatter(XmlWriterSettings writerSettings) + : this(writerSettings, loggerFactory: null) + { + } + + /// + /// Initializes a new instance of . + /// + /// The settings to be used by the . + /// The . + public XmlDataContractSerializerOutputFormatter(XmlWriterSettings writerSettings, ILoggerFactory loggerFactory) { if (writerSettings == null) { @@ -57,6 +79,8 @@ namespace Microsoft.AspNetCore.Mvc.Formatters WrapperProviderFactories = new List(); WrapperProviderFactories.Add(new EnumerableWrapperProviderFactory(WrapperProviderFactories)); WrapperProviderFactories.Add(new SerializableErrorWrapperProviderFactory()); + + _logger = loggerFactory?.CreateLogger(GetType()); } /// @@ -138,8 +162,10 @@ namespace Microsoft.AspNetCore.Mvc.Formatters // If the serializer does not support this type it will throw an exception. return new DataContractSerializer(type, _serializerSettings); } - catch (Exception) + catch (Exception ex) { + _logger?.FailedToCreateDataContractSerializer(type.FullName, ex); + // We do not surface the caught exception because if CanWriteResult returns // false, then this Formatter is not picked up at all. return null; diff --git a/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/XmlSerializerOutputFormatter.cs b/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/XmlSerializerOutputFormatter.cs index 7b04317e18..469acb5231 100644 --- a/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/XmlSerializerOutputFormatter.cs +++ b/src/Microsoft.AspNetCore.Mvc.Formatters.Xml/XmlSerializerOutputFormatter.cs @@ -11,6 +11,7 @@ using System.Xml; using System.Xml.Serialization; using Microsoft.AspNetCore.Mvc.Formatters.Xml; using Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal; +using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Mvc.Formatters { @@ -21,13 +22,33 @@ namespace Microsoft.AspNetCore.Mvc.Formatters public class XmlSerializerOutputFormatter : TextOutputFormatter { private readonly ConcurrentDictionary _serializerCache = new ConcurrentDictionary(); + private readonly ILogger _logger; /// /// Initializes a new instance of - /// with default XmlWriterSettings. + /// with default . /// - public XmlSerializerOutputFormatter() : - this(FormattingUtilities.GetDefaultXmlWriterSettings()) + public XmlSerializerOutputFormatter() + : this(FormattingUtilities.GetDefaultXmlWriterSettings()) + { + } + + /// + /// Initializes a new instance of + /// with default . + /// + /// The . + public XmlSerializerOutputFormatter(ILoggerFactory loggerFactory) + : this(FormattingUtilities.GetDefaultXmlWriterSettings(), loggerFactory) + { + } + + /// + /// Initializes a new instance of . + /// + /// The settings to be used by the . + public XmlSerializerOutputFormatter(XmlWriterSettings writerSettings) + : this(writerSettings, loggerFactory: null) { } @@ -35,7 +56,8 @@ namespace Microsoft.AspNetCore.Mvc.Formatters /// Initializes a new instance of /// /// The settings to be used by the . - public XmlSerializerOutputFormatter(XmlWriterSettings writerSettings) + /// The . + public XmlSerializerOutputFormatter(XmlWriterSettings writerSettings, ILoggerFactory loggerFactory) { if (writerSettings == null) { @@ -54,6 +76,8 @@ namespace Microsoft.AspNetCore.Mvc.Formatters WrapperProviderFactories = new List(); WrapperProviderFactories.Add(new EnumerableWrapperProviderFactory(WrapperProviderFactories)); WrapperProviderFactories.Add(new SerializableErrorWrapperProviderFactory()); + + _logger = loggerFactory?.CreateLogger(GetType()); } /// @@ -114,8 +138,10 @@ namespace Microsoft.AspNetCore.Mvc.Formatters // If the serializer does not support this type it will throw an exception. return new XmlSerializer(type); } - catch (Exception) + catch (Exception ex) { + _logger?.FailedToCreateXmlSerializer(type.FullName, ex); + // We do not surface the caught exception because if CanWriteResult returns // false, then this Formatter is not picked up at all. return null; diff --git a/test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test.csproj b/test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test.csproj index d46bb1a533..a02cf7ca47 100644 --- a/test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test.csproj +++ b/test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test.csproj @@ -8,6 +8,7 @@ + diff --git a/test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/XmlDataContractSerializerOutputFormatterTest.cs b/test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/XmlDataContractSerializerOutputFormatterTest.cs index 1fb1d6f6de..d2e286abfb 100644 --- a/test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/XmlDataContractSerializerOutputFormatterTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/XmlDataContractSerializerOutputFormatterTest.cs @@ -11,6 +11,8 @@ using System.Xml; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal; using Microsoft.AspNetCore.Testing.xunit; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Testing; using Microsoft.Extensions.Primitives; using Microsoft.Net.Http.Headers; using Moq; @@ -626,6 +628,60 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml XmlAssert.Equal(expectedOutput, content); } + public static TheoryData LogsWhenUnableToCreateSerializerForTypeData + { + get + { + var sink1 = new TestSink(); + var formatter1 = new XmlDataContractSerializerOutputFormatter(new TestLoggerFactory(sink1, enabled: true)); + + var sink2 = new TestSink(); + var formatter2 = new XmlDataContractSerializerOutputFormatter( + new XmlWriterSettings(), + new TestLoggerFactory(sink2, enabled: true)); + + return new TheoryData() + { + { formatter1, sink1 }, + { formatter2, sink2} + }; + } + } + + [Theory] + [MemberData(nameof(LogsWhenUnableToCreateSerializerForTypeData))] + public void CannotCreateSerializer_LogsWarning( + XmlDataContractSerializerOutputFormatter formatter, + TestSink sink) + { + // Arrange + var outputFormatterContext = GetOutputFormatterContext(new Customer(10), typeof(Customer)); + + // Act + var result = formatter.CanWriteResult(outputFormatterContext); + + // Assert + Assert.False(result); + var write = Assert.Single(sink.Writes); + Assert.Equal(LogLevel.Warning, write.LogLevel); + Assert.Equal($"An error occurred while trying to create a DataContractSerializer for the type '{typeof(Customer).FullName}'.", + write.State.ToString()); + } + + [Fact] + public void DoesNotThrow_OnNoLoggerAnd_WhenUnableToCreateSerializerForType() + { + // Arrange + var formatter = new XmlDataContractSerializerOutputFormatter(); // no logger is being supplied here on purpose + var outputFormatterContext = GetOutputFormatterContext(new Customer(10), typeof(Customer)); + + // Act + var canWriteResult = formatter.CanWriteResult(outputFormatterContext); + + // Assert + Assert.False(canWriteResult); + } + private OutputFormatterWriteContext GetOutputFormatterContext( object outputValue, Type outputType, @@ -666,5 +722,14 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml return base.CreateSerializer(type); } } + + public class Customer + { + public Customer(int id) + { + } + + public int MyProperty { get; set; } + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/XmlSerializerOutputFormatterTest.cs b/test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/XmlSerializerOutputFormatterTest.cs index 13c3ee6220..1bb613a2e9 100644 --- a/test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/XmlSerializerOutputFormatterTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/XmlSerializerOutputFormatterTest.cs @@ -7,9 +7,12 @@ using System.IO; using System.Linq; using System.Text; using System.Threading.Tasks; +using System.Xml; using System.Xml.Serialization; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Testing; using Microsoft.Extensions.Primitives; using Microsoft.Net.Http.Headers; using Moq; @@ -330,7 +333,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml { // Arrange var formatter = new XmlSerializerOutputFormatter(); - var outputFormatterContext = GetOutputFormatterContext(new object(), typeof (object)); + var outputFormatterContext = GetOutputFormatterContext(new object(), typeof(object)); outputFormatterContext.ContentType = new StringSegment(mediaType); outputFormatterContext.ContentTypeIsServerDefined = isServerDefined; @@ -389,6 +392,61 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml } } + public static TheoryData LogsWhenUnableToCreateSerializerForTypeData + { + get + { + var sink1 = new TestSink(); + var formatter1 = new XmlSerializerOutputFormatter(new TestLoggerFactory(sink1, enabled: true)); + + var sink2 = new TestSink(); + var formatter2 = new XmlSerializerOutputFormatter( + new XmlWriterSettings(), + new TestLoggerFactory(sink2, enabled: true)); + + return new TheoryData() + { + { formatter1, sink1 }, + { formatter2, sink2} + }; + } + } + + [Theory] + [MemberData(nameof(LogsWhenUnableToCreateSerializerForTypeData))] + public void XmlSerializer_LogsWhenUnableToCreateSerializerForType( + XmlSerializerOutputFormatter formatter, + TestSink sink) + { + // Arrange + var outputFormatterContext = GetOutputFormatterContext(new Customer(10), typeof(Customer)); + + // Act + var canWriteResult = formatter.CanWriteResult(outputFormatterContext); + + // Assert + Assert.False(canWriteResult); + var write = Assert.Single(sink.Writes); + Assert.Equal(LogLevel.Warning, write.LogLevel); + Assert.Equal( + $"An error occurred while trying to create an XmlSerializer for the type '{typeof(Customer).FullName}'.", + write.State.ToString()); + } + + [Fact] + public void XmlSerializer_DoesNotThrow_OnNoLoggerAnd_WhenUnableToCreateSerializerForType() + { + // Arrange + var formatter = new XmlSerializerOutputFormatter(); // no logger is being supplied here on purpose + var outputFormatterContext = GetOutputFormatterContext(new Customer(10), typeof(Customer)); + + // Act + var canWriteResult = formatter.CanWriteResult(outputFormatterContext); + + // Assert + Assert.False(canWriteResult); + } + private OutputFormatterWriteContext GetOutputFormatterContext( object outputValue, Type outputType, @@ -429,5 +487,14 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml return base.CreateSerializer(type); } } + + public class Customer + { + public Customer(int id) + { + } + + public int MyProperty { get; set; } + } } } \ No newline at end of file