From 7003fb53d542de6f4bf999f2edaa91c6951e1be0 Mon Sep 17 00:00:00 2001 From: Kristian Hellang Date: Wed, 26 Jun 2019 00:07:35 +0200 Subject: [PATCH] Add explicit content types to 'no formatter' logging (#11213) * Add explicit content types to 'no formatter' logging * Remove dead code --- .../DefaultOutputFormatterSelector.cs | 7 ++-- .../Infrastructure/ObjectResultExecutor.cs | 2 +- .../Mvc.Core/src/MvcCoreLoggerExtensions.cs | 21 +++++++----- .../test/MvcCoreLoggerExtensionsTest.cs | 33 +++++++++++++++++++ 4 files changed, 49 insertions(+), 14 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/Infrastructure/DefaultOutputFormatterSelector.cs b/src/Mvc/Mvc.Core/src/Infrastructure/DefaultOutputFormatterSelector.cs index bafe6d239d..d26d7decb4 100644 --- a/src/Mvc/Mvc.Core/src/Infrastructure/DefaultOutputFormatterSelector.cs +++ b/src/Mvc/Mvc.Core/src/Infrastructure/DefaultOutputFormatterSelector.cs @@ -147,14 +147,11 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure } } - if (selectedFormatter == null) + if (selectedFormatter != null) { - // No formatter supports this. - _logger.NoFormatter(context); - return null; + _logger.FormatterSelected(selectedFormatter, context); } - _logger.FormatterSelected(selectedFormatter, context); return selectedFormatter; } diff --git a/src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs b/src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs index 3dd2ddc845..8f2a40bbfc 100644 --- a/src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs +++ b/src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs @@ -148,7 +148,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure if (selectedFormatter == null) { // No formatter supports this. - Logger.NoFormatter(formatterContext); + Logger.NoFormatter(formatterContext, result.ContentTypes); context.HttpContext.Response.StatusCode = StatusCodes.Status406NotAcceptable; return Task.CompletedTask; diff --git a/src/Mvc/Mvc.Core/src/MvcCoreLoggerExtensions.cs b/src/Mvc/Mvc.Core/src/MvcCoreLoggerExtensions.cs index 26534c1712..17a94f3edf 100644 --- a/src/Mvc/Mvc.Core/src/MvcCoreLoggerExtensions.cs +++ b/src/Mvc/Mvc.Core/src/MvcCoreLoggerExtensions.cs @@ -4,7 +4,6 @@ using System; using System.Collections; using System.Collections.Generic; -using System.Diagnostics; using System.Linq; using System.Reflection; using System.Security.Claims; @@ -28,8 +27,6 @@ namespace Microsoft.AspNetCore.Mvc public const string ActionFilter = "Action Filter"; private static readonly string[] _noFilters = new[] { "None" }; - private static readonly double TimestampToTicks = TimeSpan.TicksPerSecond / (double)Stopwatch.Frequency; - private static readonly Action _actionExecuting; private static readonly Action _controllerActionExecuting; private static readonly Action _actionExecuted; @@ -73,7 +70,7 @@ namespace Microsoft.AspNetCore.Mvc private static readonly Action _localRedirectResultExecuting; private static readonly Action _objectResultExecuting; - private static readonly Action _noFormatter; + private static readonly Action, Exception> _noFormatter; private static readonly Action _formatterSelected; private static readonly Action _skippedContentNegotiation; private static readonly Action _noAcceptForNegotiation; @@ -300,10 +297,10 @@ namespace Microsoft.AspNetCore.Mvc new EventId(1, "LocalRedirectResultExecuting"), "Executing LocalRedirectResult, redirecting to {Destination}."); - _noFormatter = LoggerMessage.Define( + _noFormatter = LoggerMessage.Define>( LogLevel.Warning, new EventId(1, "NoFormatter"), - "No output formatter was found for content type '{ContentType}' to write the response."); + "No output formatter was found for content types '{ContentTypes}' to write the response."); _objectResultExecuting = LoggerMessage.Define( LogLevel.Information, @@ -1017,11 +1014,19 @@ namespace Microsoft.AspNetCore.Mvc public static void NoFormatter( this ILogger logger, - OutputFormatterCanWriteContext formatterContext) + OutputFormatterCanWriteContext context, + MediaTypeCollection contentTypes) { if (logger.IsEnabled(LogLevel.Warning)) { - _noFormatter(logger, Convert.ToString(formatterContext.ContentType), null); + var considered = new List(contentTypes); + + if (context.ContentType.HasValue) + { + considered.Add(Convert.ToString(context.ContentType)); + } + + _noFormatter(logger, considered, null); } } diff --git a/src/Mvc/Mvc.Core/test/MvcCoreLoggerExtensionsTest.cs b/src/Mvc/Mvc.Core/test/MvcCoreLoggerExtensionsTest.cs index ae6987a4c4..37688c2107 100644 --- a/src/Mvc/Mvc.Core/test/MvcCoreLoggerExtensionsTest.cs +++ b/src/Mvc/Mvc.Core/test/MvcCoreLoggerExtensionsTest.cs @@ -1,7 +1,9 @@ // 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.Http; using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.Extensions.Logging.Testing; using Moq; using Xunit; @@ -282,6 +284,37 @@ namespace Microsoft.AspNetCore.Mvc write.State.ToString()); } + [Fact] + public void NoFormatter_LogsListOfContentTypes() + { + // Arrange + var testSink = new TestSink(); + var loggerFactory = new TestLoggerFactory(testSink, enabled: true); + var logger = loggerFactory.CreateLogger("test"); + + var mediaTypes = new MediaTypeCollection + { + "application/problem+json", + "application/problem+xml", + }; + + var httpContext = Mock.Of(); + var context = new Mock(httpContext); + + context.SetupGet(x => x.ContentType).Returns("application/json"); + + // Act + logger.NoFormatter(context.Object, mediaTypes); + + // Assert + var write = Assert.Single(testSink.Writes); + Assert.Equal( + "No output formatter was found for content types " + + "'application/problem+json, application/problem+xml, application/json'" + + " to write the response.", + write.State.ToString()); + } + public interface IOrderedAuthorizeFilter : IAuthorizationFilter, IAsyncAuthorizationFilter, IOrderedFilter { } public interface IOrderedResourceFilter : IResourceFilter, IAsyncResourceFilter, IOrderedFilter { }