Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

Commit 05d02e7

Browse files
kichallakichalla
committed
[Fixes #6533] Log when XML formatters fail to create a serializer
1 parent db38da7 commit 05d02e7

8 files changed

+270
-12
lines changed

‎src/Microsoft.AspNetCore.Mvc.Formatters.Xml/Internal/MvcXmlDataContractSerializerMvcOptionsSetup.cs

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4-
using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
4+
using System;
55
using Microsoft.AspNetCore.Mvc.ModelBinding;
6+
using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
7+
using Microsoft.Extensions.Logging;
68
using Microsoft.Extensions.Options;
79

810
namespace Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal
@@ -13,6 +15,22 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal
1315
/// </summary>
1416
public class MvcXmlDataContractSerializerMvcOptionsSetup : IConfigureOptions<MvcOptions>
1517
{
18+
private readonly ILoggerFactory _loggerFactory;
19+
20+
/// <summary>
21+
/// Initializes a new instance of <see cref="MvcXmlDataContractSerializerMvcOptionsSetup"/>.
22+
/// </summary>
23+
/// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
24+
public MvcXmlDataContractSerializerMvcOptionsSetup(ILoggerFactory loggerFactory)
25+
{
26+
if (loggerFactory == null)
27+
{
28+
throw new ArgumentNullException(nameof(loggerFactory));
29+
}
30+
31+
_loggerFactory = loggerFactory;
32+
}
33+
1634
/// <summary>
1735
/// Adds the data contract serializer formatters to <see cref="MvcOptions"/>.
1836
/// </summary>
@@ -21,7 +39,7 @@ public void Configure(MvcOptions options)
2139
{
2240
options.ModelMetadataDetailsProviders.Add(new DataMemberRequiredBindingMetadataProvider());
2341

24-
options.OutputFormatters.Add(new XmlDataContractSerializerOutputFormatter());
42+
options.OutputFormatters.Add(new XmlDataContractSerializerOutputFormatter(_loggerFactory));
2543
options.InputFormatters.Add(new XmlDataContractSerializerInputFormatter(options));
2644

2745
options.ModelMetadataDetailsProviders.Add(new SuppressChildValidationMetadataProvider("System.Xml.Linq.XObject"));

‎src/Microsoft.AspNetCore.Mvc.Formatters.Xml/Internal/MvcXmlSerializerMvcOptionsSetup.cs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4+
using System;
5+
using Microsoft.Extensions.Logging;
46
using Microsoft.Extensions.Options;
57

68
namespace Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal
@@ -11,13 +13,29 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal
1113
/// </summary>
1214
public class MvcXmlSerializerMvcOptionsSetup : IConfigureOptions<MvcOptions>
1315
{
16+
private readonly ILoggerFactory _loggerFactory;
17+
18+
/// <summary>
19+
/// Initializes a new instance of <see cref="MvcXmlSerializerMvcOptionsSetup"/>.
20+
/// </summary>
21+
/// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
22+
public MvcXmlSerializerMvcOptionsSetup(ILoggerFactory loggerFactory)
23+
{
24+
if (loggerFactory == null)
25+
{
26+
throw new ArgumentNullException(nameof(loggerFactory));
27+
}
28+
29+
_loggerFactory = loggerFactory;
30+
}
31+
1432
/// <summary>
1533
/// Adds the XML serializer formatters to <see cref="MvcOptions"/>.
1634
/// </summary>
1735
/// <param name="options">The <see cref="MvcOptions"/>.</param>
1836
public void Configure(MvcOptions options)
1937
{
20-
options.OutputFormatters.Add(new XmlSerializerOutputFormatter());
38+
options.OutputFormatters.Add(new XmlSerializerOutputFormatter(_loggerFactory));
2139
options.InputFormatters.Add(new XmlSerializerInputFormatter(options));
2240
}
2341
}

‎src/Microsoft.AspNetCore.Mvc.Formatters.Xml/LoggerExtensions.cs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System;
5+
using Microsoft.Extensions.Logging;
6+
7+
namespace Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal
8+
{
9+
public static class LoggerExtensions
10+
{
11+
private static readonly Action<ILogger, string, Exception> _failedToCreateXmlSerializer;
12+
private static readonly Action<ILogger, string, Exception> _failedToCreateDataContractSerializer;
13+
14+
static LoggerExtensions()
15+
{
16+
_failedToCreateXmlSerializer = LoggerMessage.Define<string>(
17+
LogLevel.Warning,
18+
1,
19+
"An error occurred while trying to create an XmlSerializer for the type '{Type}'.");
20+
21+
_failedToCreateDataContractSerializer = LoggerMessage.Define<string>(
22+
LogLevel.Warning,
23+
2,
24+
"An error occurred while trying to create a DataContractSerializer for the type '{Type}'.");
25+
}
26+
27+
public static void FailedToCreateXmlSerializer(this ILogger logger, string typeName, Exception exception)
28+
{
29+
_failedToCreateXmlSerializer(logger, typeName, exception);
30+
}
31+
32+
public static void FailedToCreateDataContractSerializer(this ILogger logger, string typeName, Exception exception)
33+
{
34+
_failedToCreateDataContractSerializer(logger, typeName, exception);
35+
}
36+
}
37+
}

‎src/Microsoft.AspNetCore.Mvc.Formatters.Xml/XmlDataContractSerializerOutputFormatter.cs

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using System.Xml;
1212
using Microsoft.AspNetCore.Mvc.Formatters.Xml;
1313
using Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal;
14+
using Microsoft.Extensions.Logging;
1415

1516
namespace Microsoft.AspNetCore.Mvc.Formatters
1617
{
@@ -21,22 +22,43 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
2122
public class XmlDataContractSerializerOutputFormatter : TextOutputFormatter
2223
{
2324
private readonly ConcurrentDictionary<Type, object> _serializerCache = new ConcurrentDictionary<Type, object>();
25+
private readonly ILogger _logger;
2426
private DataContractSerializerSettings _serializerSettings;
2527

2628
/// <summary>
2729
/// Initializes a new instance of <see cref="XmlDataContractSerializerOutputFormatter"/>
28-
/// with default XmlWriterSettings
30+
/// with default <see cref="XmlWriterSettings"/>.
2931
/// </summary>
30-
public XmlDataContractSerializerOutputFormatter() :
31-
this(FormattingUtilities.GetDefaultXmlWriterSettings())
32+
public XmlDataContractSerializerOutputFormatter()
33+
: this(FormattingUtilities.GetDefaultXmlWriterSettings())
3234
{
3335
}
3436

3537
/// <summary>
3638
/// Initializes a new instance of <see cref="XmlDataContractSerializerOutputFormatter"/>
39+
/// with default <see cref="XmlWriterSettings"/>.
40+
/// </summary>
41+
/// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
42+
public XmlDataContractSerializerOutputFormatter(ILoggerFactory loggerFactory)
43+
: this(FormattingUtilities.GetDefaultXmlWriterSettings(), loggerFactory)
44+
{
45+
}
46+
47+
/// <summary>
48+
/// Initializes a new instance of <see cref="XmlDataContractSerializerOutputFormatter"/>.
3749
/// </summary>
3850
/// <param name="writerSettings">The settings to be used by the <see cref="DataContractSerializer"/>.</param>
3951
public XmlDataContractSerializerOutputFormatter(XmlWriterSettings writerSettings)
52+
: this(writerSettings, loggerFactory: null)
53+
{
54+
}
55+
56+
/// <summary>
57+
/// Initializes a new instance of <see cref="XmlDataContractSerializerOutputFormatter"/>.
58+
/// </summary>
59+
/// <param name="writerSettings">The settings to be used by the <see cref="DataContractSerializer"/>.</param>
60+
/// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
61+
public XmlDataContractSerializerOutputFormatter(XmlWriterSettings writerSettings, ILoggerFactory loggerFactory)
4062
{
4163
if (writerSettings == null)
4264
{
@@ -57,6 +79,8 @@ public XmlDataContractSerializerOutputFormatter(XmlWriterSettings writerSettings
5779
WrapperProviderFactories = new List<IWrapperProviderFactory>();
5880
WrapperProviderFactories.Add(new EnumerableWrapperProviderFactory(WrapperProviderFactories));
5981
WrapperProviderFactories.Add(new SerializableErrorWrapperProviderFactory());
82+
83+
_logger = loggerFactory?.CreateLogger(GetType());
6084
}
6185

6286
/// <summary>
@@ -138,8 +162,10 @@ protected virtual DataContractSerializer CreateSerializer(Type type)
138162
// If the serializer does not support this type it will throw an exception.
139163
return new DataContractSerializer(type, _serializerSettings);
140164
}
141-
catch (Exception)
165+
catch (Exception ex)
142166
{
167+
_logger?.FailedToCreateDataContractSerializer(type.FullName, ex);
168+
143169
// We do not surface the caught exception because if CanWriteResult returns
144170
// false, then this Formatter is not picked up at all.
145171
return null;

‎src/Microsoft.AspNetCore.Mvc.Formatters.Xml/XmlSerializerOutputFormatter.cs

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using System.Xml.Serialization;
1212
using Microsoft.AspNetCore.Mvc.Formatters.Xml;
1313
using Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal;
14+
using Microsoft.Extensions.Logging;
1415

1516
namespace Microsoft.AspNetCore.Mvc.Formatters
1617
{
@@ -21,21 +22,42 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
2122
public class XmlSerializerOutputFormatter : TextOutputFormatter
2223
{
2324
private readonly ConcurrentDictionary<Type, object> _serializerCache = new ConcurrentDictionary<Type, object>();
25+
private readonly ILogger _logger;
2426

2527
/// <summary>
2628
/// Initializes a new instance of <see cref="XmlSerializerOutputFormatter"/>
27-
/// with default XmlWriterSettings.
29+
/// with default <see cref="XmlWriterSettings"/>.
2830
/// </summary>
29-
public XmlSerializerOutputFormatter() :
30-
this(FormattingUtilities.GetDefaultXmlWriterSettings())
31+
public XmlSerializerOutputFormatter()
32+
: this(FormattingUtilities.GetDefaultXmlWriterSettings())
3133
{
3234
}
3335

3436
/// <summary>
3537
/// Initializes a new instance of <see cref="XmlSerializerOutputFormatter"/>
38+
/// with default <see cref="XmlWriterSettings"/>.
39+
/// </summary>
40+
/// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
41+
public XmlSerializerOutputFormatter(ILoggerFactory loggerFactory)
42+
: this(FormattingUtilities.GetDefaultXmlWriterSettings(), loggerFactory)
43+
{
44+
}
45+
46+
/// <summary>
47+
/// Initializes a new instance of <see cref="XmlSerializerOutputFormatter"/>.
3648
/// </summary>
3749
/// <param name="writerSettings">The settings to be used by the <see cref="XmlSerializer"/>.</param>
3850
public XmlSerializerOutputFormatter(XmlWriterSettings writerSettings)
51+
: this(writerSettings, loggerFactory: null)
52+
{
53+
}
54+
55+
/// <summary>
56+
/// Initializes a new instance of <see cref="XmlSerializerOutputFormatter"/>
57+
/// </summary>
58+
/// <param name="writerSettings">The settings to be used by the <see cref="XmlSerializer"/>.</param>
59+
/// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
60+
public XmlSerializerOutputFormatter(XmlWriterSettings writerSettings, ILoggerFactory loggerFactory)
3961
{
4062
if (writerSettings == null)
4163
{
@@ -54,6 +76,8 @@ public XmlSerializerOutputFormatter(XmlWriterSettings writerSettings)
5476
WrapperProviderFactories = new List<IWrapperProviderFactory>();
5577
WrapperProviderFactories.Add(new EnumerableWrapperProviderFactory(WrapperProviderFactories));
5678
WrapperProviderFactories.Add(new SerializableErrorWrapperProviderFactory());
79+
80+
_logger = loggerFactory?.CreateLogger(GetType());
5781
}
5882

5983
/// <summary>
@@ -114,8 +138,10 @@ protected virtual XmlSerializer CreateSerializer(Type type)
114138
// If the serializer does not support this type it will throw an exception.
115139
return new XmlSerializer(type);
116140
}
117-
catch (Exception)
141+
catch (Exception ex)
118142
{
143+
_logger?.FailedToCreateXmlSerializer(type.FullName, ex);
144+
119145
// We do not surface the caught exception because if CanWriteResult returns
120146
// false, then this Formatter is not picked up at all.
121147
return null;

‎test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
<ProjectReference Include="..\Microsoft.AspNetCore.Mvc.TestCommon\Microsoft.AspNetCore.Mvc.TestCommon.csproj" />
99

1010
<PackageReference Include="Microsoft.AspNetCore.Http" Version="$(MicrosoftAspNetCoreHttpPackageVersion)" />
11+
<PackageReference Include="Microsoft.Extensions.Logging.Testing" Version="$(MicrosoftExtensionsLoggingTestingPackageVersion)" />
1112
</ItemGroup>
1213

1314
</Project>

‎test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/XmlDataContractSerializerOutputFormatterTest.cs

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
using Microsoft.AspNetCore.Http;
1212
using Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal;
1313
using Microsoft.AspNetCore.Testing.xunit;
14+
using Microsoft.Extensions.Logging;
15+
using Microsoft.Extensions.Logging.Testing;
1416
using Microsoft.Extensions.Primitives;
1517
using Microsoft.Net.Http.Headers;
1618
using Moq;
@@ -626,6 +628,60 @@ public async Task WriteAsync_WritesWhenConfiguredWithPreserveReferences()
626628
XmlAssert.Equal(expectedOutput, content);
627629
}
628630

631+
public static TheoryData<XmlDataContractSerializerOutputFormatter, TestSink> LogsWhenUnableToCreateSerializerForTypeData
632+
{
633+
get
634+
{
635+
var sink1 = new TestSink();
636+
var formatter1 = new XmlDataContractSerializerOutputFormatter(new TestLoggerFactory(sink1, enabled: true));
637+
638+
var sink2 = new TestSink();
639+
var formatter2 = new XmlDataContractSerializerOutputFormatter(
640+
new XmlWriterSettings(),
641+
new TestLoggerFactory(sink2, enabled: true));
642+
643+
return new TheoryData<XmlDataContractSerializerOutputFormatter, TestSink>()
644+
{
645+
{ formatter1, sink1 },
646+
{ formatter2, sink2}
647+
};
648+
}
649+
}
650+
651+
[Theory]
652+
[MemberData(nameof(LogsWhenUnableToCreateSerializerForTypeData))]
653+
public void CannotCreateSerializer_LogsWarning(
654+
XmlDataContractSerializerOutputFormatter formatter,
655+
TestSink sink)
656+
{
657+
// Arrange
658+
var outputFormatterContext = GetOutputFormatterContext(new Customer(10), typeof(Customer));
659+
660+
// Act
661+
var result = formatter.CanWriteResult(outputFormatterContext);
662+
663+
// Assert
664+
Assert.False(result);
665+
var write = Assert.Single(sink.Writes);
666+
Assert.Equal(LogLevel.Warning, write.LogLevel);
667+
Assert.Equal($"An error occurred while trying to create a DataContractSerializer for the type '{typeof(Customer).FullName}'.",
668+
write.State.ToString());
669+
}
670+
671+
[Fact]
672+
public void DoesNotThrow_OnNoLoggerAnd_WhenUnableToCreateSerializerForType()
673+
{
674+
// Arrange
675+
var formatter = new XmlDataContractSerializerOutputFormatter(); // no logger is being supplied here on purpose
676+
var outputFormatterContext = GetOutputFormatterContext(new Customer(10), typeof(Customer));
677+
678+
// Act
679+
var canWriteResult = formatter.CanWriteResult(outputFormatterContext);
680+
681+
// Assert
682+
Assert.False(canWriteResult);
683+
}
684+
629685
private OutputFormatterWriteContext GetOutputFormatterContext(
630686
object outputValue,
631687
Type outputType,
@@ -666,5 +722,14 @@ protected override DataContractSerializer CreateSerializer(Type type)
666722
return base.CreateSerializer(type);
667723
}
668724
}
725+
726+
public class Customer
727+
{
728+
public Customer(int id)
729+
{
730+
}
731+
732+
public int MyProperty { get; set; }
733+
}
669734
}
670735
}

0 commit comments

Comments
 (0)