From 5763eb580a85c1a197874070ccbac3e0e733b845 Mon Sep 17 00:00:00 2001 From: ryanbrandenburg Date: Wed, 21 Oct 2015 16:06:56 -0700 Subject: [PATCH] * Move logging to new style --- .../Controllers/FilterActionInvoker.cs | 42 ++----- .../Infrastructure/DefaultActionSelector.cs | 10 +- .../Infrastructure/ObjectResultExecutor.cs | 17 +-- .../ChallengeResultLoggerExtensions.cs | 4 +- .../Logging/ContentResultLoggerExtensions.cs | 4 +- ...ControllerActionInvokerLoggerExtensions.cs | 6 +- .../DefaultActionSelectorLoggerExtensions.cs | 41 +++++++ .../Logging/FileResultLoggerExtensions.cs | 4 +- .../FilterActionInvokerLoggerExtensions.cs | 65 +++++++++++ .../HttpStatusCodeResultLoggerExtensions.cs | 4 +- .../InnerAttributeRouteLoggerExtensions.cs | 26 +++++ .../LocalRedirectResultLoggerExtensions.cs | 4 +- .../MvcRouteHandlerLoggerExtensions.cs | 4 +- .../ObjectResultExecutorLoggerExtensions.cs | 65 ++++++++++- .../Logging/RedirectResultLoggerExtensions.cs | 4 +- .../RedirectToActionResultLoggerExtension.cs | 4 +- .../RedirectToRouteResultLoggerExtensions.cs | 4 +- .../ViewResultExecutorLoggerExtensions.cs | 37 +++++++ .../Routing/InnerAttributeRoute.cs | 4 +- .../Logging/JsonResultLoggerExtensions.cs | 4 +- .../Internal/ModeMatchResult.cs | 13 +-- .../Internal/PartialModeMatchLogValuesOfT.cs | 63 ----------- .../ModeMatchResultLoggerExtensions.cs | 103 ++++++++++++++++++ ...rtialViewResultExecutorLoggerExtensions.cs | 32 +++++- .../ViewFeatures/PartialViewResultExecutor.cs | 8 +- .../ViewFeatures/ViewResultExecutor.cs | 10 +- .../RazorTextWriterTest.cs | 6 +- 27 files changed, 422 insertions(+), 166 deletions(-) create mode 100644 src/Microsoft.AspNet.Mvc.Core/Logging/DefaultActionSelectorLoggerExtensions.cs create mode 100644 src/Microsoft.AspNet.Mvc.Core/Logging/FilterActionInvokerLoggerExtensions.cs create mode 100644 src/Microsoft.AspNet.Mvc.Core/Logging/InnerAttributeRouteLoggerExtensions.cs create mode 100644 src/Microsoft.AspNet.Mvc.Core/Logging/ViewResultExecutorLoggerExtensions.cs rename src/{Microsoft.AspNet.Mvc.Core => Microsoft.AspNet.Mvc.Formatters.Json}/Logging/JsonResultLoggerExtensions.cs (83%) delete mode 100644 src/Microsoft.AspNet.Mvc.TagHelpers/Internal/PartialModeMatchLogValuesOfT.cs create mode 100644 src/Microsoft.AspNet.Mvc.TagHelpers/Logging/ModeMatchResultLoggerExtensions.cs diff --git a/src/Microsoft.AspNet.Mvc.Core/Controllers/FilterActionInvoker.cs b/src/Microsoft.AspNet.Mvc.Core/Controllers/FilterActionInvoker.cs index e90e8ea574..3bc87fff0f 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Controllers/FilterActionInvoker.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Controllers/FilterActionInvoker.cs @@ -13,6 +13,7 @@ using Microsoft.AspNet.Mvc.Filters; using Microsoft.AspNet.Mvc.Formatters; using Microsoft.AspNet.Mvc.Infrastructure; using Microsoft.AspNet.Mvc.Internal; +using Microsoft.AspNet.Mvc.Logging; using Microsoft.AspNet.Mvc.ModelBinding; using Microsoft.AspNet.Mvc.ModelBinding.Validation; using Microsoft.Extensions.Logging; @@ -48,17 +49,6 @@ namespace Microsoft.AspNet.Mvc.Controllers private ResultExecutingContext _resultExecutingContext; private ResultExecutedContext _resultExecutedContext; - - private const string AuthorizationFailureLogMessage = - "Authorization failed for the request at filter '{AuthorizationFilter}'."; - private const string ResourceFilterShortCircuitLogMessage = - "Request was short circuited at resource filter '{ResourceFilter}'."; - private const string ActionFilterShortCircuitLogMessage = - "Request was short circuited at action filter '{ActionFilter}'."; - private const string ExceptionFilterShortCircuitLogMessage = - "Request was short circuited at exception filter '{ExceptionFilter}'."; - private const string ResultFilterShortCircuitLogMessage = - "Request was short circuited at result filter '{ResultFilter}'."; public FilterActionInvoker( ActionContext actionContext, @@ -306,7 +296,7 @@ namespace Microsoft.AspNet.Mvc.Controllers } else { - Logger.LogWarning(AuthorizationFailureLogMessage, current.FilterAsync.GetType().FullName); + Logger.AuthorizationFailure(current.FilterAsync); } } else if (current.Filter != null) @@ -328,7 +318,7 @@ namespace Microsoft.AspNet.Mvc.Controllers } else { - Logger.LogWarning(AuthorizationFailureLogMessage, current.Filter.GetType().FullName); + Logger.AuthorizationFailure(current.Filter); } } else @@ -393,9 +383,7 @@ namespace Microsoft.AspNet.Mvc.Controllers // If we get here then the filter didn't call 'next' indicating a short circuit if (_resourceExecutingContext.Result != null) { - Logger.LogVerbose( - ResourceFilterShortCircuitLogMessage, - item.FilterAsync.GetType().FullName); + Logger.ResourceFilterShortCircuited(item.FilterAsync); await InvokeResultAsync(_resourceExecutingContext.Result); } @@ -422,8 +410,8 @@ namespace Microsoft.AspNet.Mvc.Controllers if (_resourceExecutingContext.Result != null) { // Short-circuited by setting a result. - Logger.LogVerbose(ResourceFilterShortCircuitLogMessage, item.Filter.GetType().FullName); - + Logger.ResourceFilterShortCircuited(item.Filter); + await InvokeResultAsync(_resourceExecutingContext.Result); _resourceExecutedContext = new ResourceExecutedContext(_resourceExecutingContext, _filters) @@ -558,9 +546,7 @@ namespace Microsoft.AspNet.Mvc.Controllers if (_exceptionContext.Exception == null) { - Logger.LogVerbose( - ExceptionFilterShortCircuitLogMessage, - current.FilterAsync.GetType().FullName); + Logger.ExceptionFilterShortCircuited(current.FilterAsync); } } } @@ -587,9 +573,7 @@ namespace Microsoft.AspNet.Mvc.Controllers if (_exceptionContext.Exception == null) { - Logger.LogVerbose( - ExceptionFilterShortCircuitLogMessage, - current.Filter.GetType().FullName); + Logger.ExceptionFilterShortCircuited(current.Filter); } } } @@ -676,8 +660,7 @@ namespace Microsoft.AspNet.Mvc.Controllers if (_actionExecutedContext == null) { // If we get here then the filter didn't call 'next' indicating a short circuit - - Logger.LogVerbose(ActionFilterShortCircuitLogMessage, item.FilterAsync.GetType().FullName); + Logger.ActionFilterShortCircuited(item.FilterAsync); _actionExecutedContext = new ActionExecutedContext( _actionExecutingContext, @@ -704,8 +687,7 @@ namespace Microsoft.AspNet.Mvc.Controllers if (_actionExecutingContext.Result != null) { // Short-circuited by setting a result. - - Logger.LogVerbose(ActionFilterShortCircuitLogMessage, item.Filter.GetType().FullName); + Logger.ActionFilterShortCircuited(item.Filter); _actionExecutedContext = new ActionExecutedContext( _actionExecutingContext, @@ -834,7 +816,7 @@ namespace Microsoft.AspNet.Mvc.Controllers if (_resultExecutedContext == null || _resultExecutingContext.Cancel == true) { // Short-circuited by not calling next || Short-circuited by setting Cancel == true - Logger.LogVerbose(ResourceFilterShortCircuitLogMessage, item.FilterAsync.GetType().FullName); + Logger.ResourceFilterShortCircuited(item.FilterAsync); _resultExecutedContext = new ResultExecutedContext( _resultExecutingContext, @@ -861,7 +843,7 @@ namespace Microsoft.AspNet.Mvc.Controllers if (_resultExecutingContext.Cancel == true) { // Short-circuited by setting Cancel == true - Logger.LogVerbose(ResourceFilterShortCircuitLogMessage, item.Filter.GetType().FullName); + Logger.ResourceFilterShortCircuited(item.Filter); _resultExecutedContext = new ResultExecutedContext( _resultExecutingContext, diff --git a/src/Microsoft.AspNet.Mvc.Core/Infrastructure/DefaultActionSelector.cs b/src/Microsoft.AspNet.Mvc.Core/Infrastructure/DefaultActionSelector.cs index e9c5c62d51..4bcee52b14 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Infrastructure/DefaultActionSelector.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Infrastructure/DefaultActionSelector.cs @@ -9,6 +9,7 @@ using Microsoft.AspNet.Http; using Microsoft.AspNet.Mvc.Abstractions; using Microsoft.AspNet.Mvc.ActionConstraints; using Microsoft.AspNet.Mvc.Core; +using Microsoft.AspNet.Mvc.Logging; using Microsoft.AspNet.Mvc.Routing; using Microsoft.AspNet.Routing; using Microsoft.Extensions.Logging; @@ -87,8 +88,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure Environment.NewLine, finalMatches.Select(a => a.DisplayName)); - _logger.LogError("Request matched multiple actions resulting in ambiguity. " + - "Matching actions: {AmbiguousActions}", actionNames); + _logger.AmbiguousActions(actionNames); var message = Resources.FormatDefaultActionSelector_AmbiguousActions( Environment.NewLine, @@ -170,14 +170,10 @@ namespace Microsoft.AspNet.Mvc.Infrastructure if (!constraint.Accept(constraintContext)) { isMatch = false; - - _logger.LogVerbose( - "Action '{ActionDisplayName}' with id '{ActionId}' did not match the " + - "constraint '{ActionConstraint}'", + _logger.ConstraintMismatch( candidate.Action.DisplayName, candidate.Action.Id, constraint); - break; } } diff --git a/src/Microsoft.AspNet.Mvc.Core/Infrastructure/ObjectResultExecutor.cs b/src/Microsoft.AspNet.Mvc.Core/Infrastructure/ObjectResultExecutor.cs index 6d3a627c09..8d942fd0d3 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Infrastructure/ObjectResultExecutor.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Infrastructure/ObjectResultExecutor.cs @@ -128,18 +128,13 @@ namespace Microsoft.AspNet.Mvc.Infrastructure if (selectedFormatter == null) { // No formatter supports this. - Logger.LogWarning("No output formatter was found to write the response."); + Logger.NoFormatter(formatterContext); context.HttpContext.Response.StatusCode = StatusCodes.Status406NotAcceptable; return TaskCache.CompletedTask; } - Logger.LogVerbose( - "Selected output formatter '{OutputFormatter}' and content type " + - "'{ContentType}' to write the response.", - selectedFormatter.GetType().FullName, - formatterContext.ContentType); - + Logger.FormatterSelected(selectedFormatter, formatterContext); Logger.ObjectResultExecuting(context); result.OnFormatting(context); @@ -183,9 +178,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure // or URL path extension mapping). If yes, then ignore content-negotiation and use this content-type. if (contentTypes.Count == 1) { - Logger.LogVerbose( - "Skipped content negotiation as content type '{ContentType}' is explicitly set for the response.", - contentTypes[0]); + Logger.SkippedContentNegotiation(contentTypes[0]); return SelectFormatterUsingAnyAcceptableContentType(formatterContext, formatters, contentTypes); } @@ -200,7 +193,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure // which can write the type. Let the formatter choose the Content-Type. if (acceptValues == null || acceptValues.Count == 0) { - Logger.LogVerbose("No information found on request to perform content negotiation."); + Logger.NoAcceptForNegotiation(); return SelectFormatterNotUsingAcceptHeaders(formatterContext, formatters); } @@ -219,7 +212,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure // the type. Let the formatter choose the Content-Type. if (selectedFormatter == null) { - Logger.LogVerbose("Could not find an output formatter based on content negotiation."); + Logger.NoFormatterFromNegotiation(acceptValues); // Set this flag to indicate that content-negotiation has failed to let formatters decide // if they want to write the response or not. diff --git a/src/Microsoft.AspNet.Mvc.Core/Logging/ChallengeResultLoggerExtensions.cs b/src/Microsoft.AspNet.Mvc.Core/Logging/ChallengeResultLoggerExtensions.cs index 189b15e2b6..e1f2cba77a 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Logging/ChallengeResultLoggerExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Logging/ChallengeResultLoggerExtensions.cs @@ -8,9 +8,9 @@ using Microsoft.Extensions.Logging; namespace Microsoft.AspNet.Mvc.Logging { - public static class ChallengeResultLoggerExtenstions + internal static class ChallengeResultLoggerExtenstions { - private static Action _challengeResultExecuting; + private static readonly Action _challengeResultExecuting; static ChallengeResultLoggerExtenstions() { diff --git a/src/Microsoft.AspNet.Mvc.Core/Logging/ContentResultLoggerExtensions.cs b/src/Microsoft.AspNet.Mvc.Core/Logging/ContentResultLoggerExtensions.cs index 3ca9bdc8db..6bf3afd8b1 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Logging/ContentResultLoggerExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Logging/ContentResultLoggerExtensions.cs @@ -7,9 +7,9 @@ using Microsoft.Net.Http.Headers; namespace Microsoft.AspNet.Mvc.Logging { - public static class ContentResultLoggerExtensions + internal static class ContentResultLoggerExtensions { - private static Action _contentResultExecuting; + private static readonly Action _contentResultExecuting; static ContentResultLoggerExtensions() { diff --git a/src/Microsoft.AspNet.Mvc.Core/Logging/ControllerActionInvokerLoggerExtensions.cs b/src/Microsoft.AspNet.Mvc.Core/Logging/ControllerActionInvokerLoggerExtensions.cs index 1cfbb6008d..55cb700233 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Logging/ControllerActionInvokerLoggerExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Logging/ControllerActionInvokerLoggerExtensions.cs @@ -8,10 +8,10 @@ using Microsoft.Extensions.Logging; namespace Microsoft.AspNet.Mvc.Logging { - public static class ControllerActionInvokerLoggerExtensions + internal static class ControllerActionInvokerLoggerExtensions { - private static Action _actionMethodExecuting; - private static Action _actionMethodExecuted; + private static readonly Action _actionMethodExecuting; + private static readonly Action _actionMethodExecuted; static ControllerActionInvokerLoggerExtensions() { diff --git a/src/Microsoft.AspNet.Mvc.Core/Logging/DefaultActionSelectorLoggerExtensions.cs b/src/Microsoft.AspNet.Mvc.Core/Logging/DefaultActionSelectorLoggerExtensions.cs new file mode 100644 index 0000000000..c8c14dd056 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/Logging/DefaultActionSelectorLoggerExtensions.cs @@ -0,0 +1,41 @@ +// 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.AspNet.Mvc.ActionConstraints; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNet.Mvc.Logging +{ + internal static class DefaultActionSelectorLoggerExtensions + { + private static readonly Action _ambiguousActions; + private static readonly Action _constraintMismatch; + + static DefaultActionSelectorLoggerExtensions() + { + _ambiguousActions = LoggerMessage.Define( + LogLevel.Error, + 1, + "Request matched multiple actions resulting in ambiguity. Matching actions: {AmbiguousActions}"); + _constraintMismatch = LoggerMessage.Define( + LogLevel.Verbose, + 2, + "Action '{ActionName}' with id '{ActionId}' did not match the constraint '{ActionConstraint}'"); + } + + public static void AmbiguousActions(this ILogger logger, string actionNames) + { + _ambiguousActions(logger, actionNames, null); + } + + public static void ConstraintMismatch( + this ILogger logger, + string actionName, + string actionId, + IActionConstraint actionConstraint) + { + _constraintMismatch(logger, actionName, actionId, actionConstraint, null); + } + } +} diff --git a/src/Microsoft.AspNet.Mvc.Core/Logging/FileResultLoggerExtensions.cs b/src/Microsoft.AspNet.Mvc.Core/Logging/FileResultLoggerExtensions.cs index 6032c0afc5..8bacf7745b 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Logging/FileResultLoggerExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Logging/FileResultLoggerExtensions.cs @@ -6,9 +6,9 @@ using Microsoft.Extensions.Logging; namespace Microsoft.AspNet.Mvc.Logging { - public static class FileResultLoggerExtensions + internal static class FileResultLoggerExtensions { - private static Action _fileResultExecuting; + private static readonly Action _fileResultExecuting; static FileResultLoggerExtensions() { diff --git a/src/Microsoft.AspNet.Mvc.Core/Logging/FilterActionInvokerLoggerExtensions.cs b/src/Microsoft.AspNet.Mvc.Core/Logging/FilterActionInvokerLoggerExtensions.cs new file mode 100644 index 0000000000..663473c4bf --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/Logging/FilterActionInvokerLoggerExtensions.cs @@ -0,0 +1,65 @@ +// 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.AspNet.Mvc.Filters; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNet.Mvc.Logging +{ + internal static class FilterActionInvokerLoggerExtensions + { + private static readonly Action _authorizationFailure; + private static readonly Action _resourceFilterShortCircuit; + private static readonly Action _actionFilterShortCircuit; + private static readonly Action _exceptionFilterShortCircuit; + + static FilterActionInvokerLoggerExtensions() + { + _authorizationFailure = LoggerMessage.Define( + LogLevel.Warning, + 1, + "Authorization failed for the request at filter '{AuthorizationFilter}'."); + _resourceFilterShortCircuit = LoggerMessage.Define( + LogLevel.Verbose, + 2, + "Request was short circuited at resource filter '{ResourceFilter}'."); + _actionFilterShortCircuit = LoggerMessage.Define( + LogLevel.Verbose, + 3, + "Request was short circuited at action filter '{ActionFilter}'."); + _exceptionFilterShortCircuit = LoggerMessage.Define( + LogLevel.Verbose, + 4, + "Request was short circuited at exception filter '{ExceptionFilter}'."); + } + + public static void AuthorizationFailure( + this ILogger logger, + IFilterMetadata filter) + { + _authorizationFailure(logger, filter, null); + } + + public static void ResourceFilterShortCircuited( + this ILogger logger, + IFilterMetadata filter) + { + _resourceFilterShortCircuit(logger, filter, null); + } + + public static void ExceptionFilterShortCircuited( + this ILogger logger, + IFilterMetadata filter) + { + _exceptionFilterShortCircuit(logger, filter, null); + } + + public static void ActionFilterShortCircuited( + this ILogger logger, + IFilterMetadata filter) + { + _actionFilterShortCircuit(logger, filter, null); + } + } +} diff --git a/src/Microsoft.AspNet.Mvc.Core/Logging/HttpStatusCodeResultLoggerExtensions.cs b/src/Microsoft.AspNet.Mvc.Core/Logging/HttpStatusCodeResultLoggerExtensions.cs index ab8660ffba..514168d5f4 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Logging/HttpStatusCodeResultLoggerExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Logging/HttpStatusCodeResultLoggerExtensions.cs @@ -6,9 +6,9 @@ using Microsoft.Extensions.Logging; namespace Microsoft.AspNet.Mvc.Logging { - public static class HttpStatusCodeLoggerExtensions + internal static class HttpStatusCodeLoggerExtensions { - private static Action _httpStatusCodeResultExecuting; + private static readonly Action _httpStatusCodeResultExecuting; static HttpStatusCodeLoggerExtensions() { diff --git a/src/Microsoft.AspNet.Mvc.Core/Logging/InnerAttributeRouteLoggerExtensions.cs b/src/Microsoft.AspNet.Mvc.Core/Logging/InnerAttributeRouteLoggerExtensions.cs new file mode 100644 index 0000000000..ee3a9dc59e --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/Logging/InnerAttributeRouteLoggerExtensions.cs @@ -0,0 +1,26 @@ +using System; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNet.Mvc.Logging +{ + internal static class InnerAttributeRouteLoggerExtensions + { + private static readonly Action _matchedRouteName; + + static InnerAttributeRouteLoggerExtensions() + { + _matchedRouteName = LoggerMessage.Define( + LogLevel.Verbose, + 1, + "Request successfully matched the route with name '{RouteName}' and template '{RouteTemplate}'."); + } + + public static void MatchedRouteName( + this ILogger logger, + string routeName, + string routeTemplate) + { + _matchedRouteName(logger, routeName, routeTemplate, null); + } + } +} diff --git a/src/Microsoft.AspNet.Mvc.Core/Logging/LocalRedirectResultLoggerExtensions.cs b/src/Microsoft.AspNet.Mvc.Core/Logging/LocalRedirectResultLoggerExtensions.cs index 8764b6d354..47065347d2 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Logging/LocalRedirectResultLoggerExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Logging/LocalRedirectResultLoggerExtensions.cs @@ -6,9 +6,9 @@ using Microsoft.Extensions.Logging; namespace Microsoft.AspNet.Mvc.Logging { - public static class LocalRedirectResultLoggerExtensions + internal static class LocalRedirectResultLoggerExtensions { - private static Action _localRedirectResultExecuting; + private static readonly Action _localRedirectResultExecuting; static LocalRedirectResultLoggerExtensions() { diff --git a/src/Microsoft.AspNet.Mvc.Core/Logging/MvcRouteHandlerLoggerExtensions.cs b/src/Microsoft.AspNet.Mvc.Core/Logging/MvcRouteHandlerLoggerExtensions.cs index f6a3fee07f..4d7472d649 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Logging/MvcRouteHandlerLoggerExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Logging/MvcRouteHandlerLoggerExtensions.cs @@ -10,8 +10,8 @@ namespace Microsoft.AspNet.Mvc.Logging { internal static class MvcRouteHandlerLoggerExtensions { - private static Action _actionExecuting; - private static Action _actionExecuted; + private static readonly Action _actionExecuting; + private static readonly Action _actionExecuted; static MvcRouteHandlerLoggerExtensions() { diff --git a/src/Microsoft.AspNet.Mvc.Core/Logging/ObjectResultExecutorLoggerExtensions.cs b/src/Microsoft.AspNet.Mvc.Core/Logging/ObjectResultExecutorLoggerExtensions.cs index 5fbc39be9d..9f3e075fbc 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Logging/ObjectResultExecutorLoggerExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Logging/ObjectResultExecutorLoggerExtensions.cs @@ -1,26 +1,85 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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 System.Collections.Generic; +using Microsoft.AspNet.Mvc.Formatters; using Microsoft.Extensions.Logging; +using Microsoft.Net.Http.Headers; namespace Microsoft.AspNet.Mvc.Logging { - public static class ObjectResultExecutorLoggerExtensions + internal static class ObjectResultExecutorLoggerExtensions { - private static Action _objectResultExecuting; + private static readonly Action _objectResultExecuting; + private static readonly Action _noFormatter; + private static readonly Action _formatterSelected; + private static readonly Action _skippedContentNegotiation; + private static readonly Action _noAcceptForNegotiation; + private static readonly Action, Exception> _noFormatterFromNegotiation; static ObjectResultExecutorLoggerExtensions() { + _noFormatter = LoggerMessage.Define( + LogLevel.Warning, + 1, + "No output formatter was found for content type '{ContentType}' to write the response."); _objectResultExecuting = LoggerMessage.Define( LogLevel.Information, 1, "Executing ObjectResult, writing value {Value}."); + _formatterSelected = LoggerMessage.Define( + LogLevel.Verbose, + 2, + "Selected output formatter '{OutputFormatter}' and content type '{ContentType}' to write the response."); + _skippedContentNegotiation = LoggerMessage.Define( + LogLevel.Verbose, + 3, + "Skipped content negotiation as content type '{ContentType}' is explicitly set for the response."); + _noAcceptForNegotiation = LoggerMessage.Define( + LogLevel.Verbose, + 4, + "No information found on request to perform content negotiation."); + _noFormatterFromNegotiation = LoggerMessage.Define>( + LogLevel.Verbose, + 5, + "Could not find an output formatter based on content negotiation. Accepted types were ({AcceptTypes})"); } public static void ObjectResultExecuting(this ILogger logger, object value) { _objectResultExecuting(logger, Convert.ToString(value), null); } + + public static void NoFormatter( + this ILogger logger, + OutputFormatterWriteContext formatterContext) + { + _noFormatter(logger, Convert.ToString(formatterContext.ContentType), null); + } + + public static void FormatterSelected( + this ILogger logger, + IOutputFormatter outputFormatter, + OutputFormatterWriteContext context) + { + var contentType = Convert.ToString(context.ContentType); + _formatterSelected(logger, outputFormatter, contentType, null); + } + + public static void SkippedContentNegotiation(this ILogger logger, MediaTypeHeaderValue contentType) + { + _skippedContentNegotiation(logger, Convert.ToString(contentType), null); + } + + public static void NoAcceptForNegotiation(this ILogger logger) + { + _noAcceptForNegotiation(logger, null, null); + } + + public static void NoFormatterFromNegotiation(this ILogger logger, IEnumerable acceptTypes) + { + _noFormatterFromNegotiation(logger, acceptTypes, null); + } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/Logging/RedirectResultLoggerExtensions.cs b/src/Microsoft.AspNet.Mvc.Core/Logging/RedirectResultLoggerExtensions.cs index e91ab83be8..50c7d1951c 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Logging/RedirectResultLoggerExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Logging/RedirectResultLoggerExtensions.cs @@ -6,9 +6,9 @@ using Microsoft.Extensions.Logging; namespace Microsoft.AspNet.Mvc.Logging { - public static class RedirectResultLoggerExtensions + internal static class RedirectResultLoggerExtensions { - private static Action _redirectResultExecuting; + private static readonly Action _redirectResultExecuting; static RedirectResultLoggerExtensions() { diff --git a/src/Microsoft.AspNet.Mvc.Core/Logging/RedirectToActionResultLoggerExtension.cs b/src/Microsoft.AspNet.Mvc.Core/Logging/RedirectToActionResultLoggerExtension.cs index a19a115516..0ee6444d78 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Logging/RedirectToActionResultLoggerExtension.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Logging/RedirectToActionResultLoggerExtension.cs @@ -6,9 +6,9 @@ using Microsoft.Extensions.Logging; namespace Microsoft.AspNet.Mvc.Logging { - public static class RedirectToActionResultLoggerExtensions + internal static class RedirectToActionResultLoggerExtensions { - private static Action _redirectToActionResultExecuting; + private static readonly Action _redirectToActionResultExecuting; static RedirectToActionResultLoggerExtensions() { diff --git a/src/Microsoft.AspNet.Mvc.Core/Logging/RedirectToRouteResultLoggerExtensions.cs b/src/Microsoft.AspNet.Mvc.Core/Logging/RedirectToRouteResultLoggerExtensions.cs index cf9c2ecc90..2f8b18d273 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Logging/RedirectToRouteResultLoggerExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Logging/RedirectToRouteResultLoggerExtensions.cs @@ -6,9 +6,9 @@ using Microsoft.Extensions.Logging; namespace Microsoft.AspNet.Mvc.Logging { - public static class RedirectToRouteResultLoggerExtensions + internal static class RedirectToRouteResultLoggerExtensions { - private static Action _redirectToRouteResultExecuting; + private static readonly Action _redirectToRouteResultExecuting; static RedirectToRouteResultLoggerExtensions() { diff --git a/src/Microsoft.AspNet.Mvc.Core/Logging/ViewResultExecutorLoggerExtensions.cs b/src/Microsoft.AspNet.Mvc.Core/Logging/ViewResultExecutorLoggerExtensions.cs new file mode 100644 index 0000000000..5bbe355a12 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/Logging/ViewResultExecutorLoggerExtensions.cs @@ -0,0 +1,37 @@ +using System; +using System.Collections.Generic; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNet.Mvc.Logging +{ + internal static class ViewResultExecutorLoggerExtensions + { + private static readonly Action _viewFound; + private static readonly Action, Exception> _viewNotFound; + + static ViewResultExecutorLoggerExtensions() + { + _viewFound = LoggerMessage.Define( + LogLevel.Verbose, + 1, + "The view '{ViewName}' was found."); + _viewNotFound = LoggerMessage.Define>( + LogLevel.Error, + 2, + "The view '{ViewName}' was not found. Searched locations: {SearchedViewLocations}"); + } + + public static void ViewFound(this ILogger logger, string viewName) + { + _viewFound(logger, viewName, null); + } + + public static void ViewNotFound( + this ILogger logger, + string viewName, + IEnumerable searchedLocations) + { + _viewNotFound(logger, viewName, searchedLocations, null); + } + } +} diff --git a/src/Microsoft.AspNet.Mvc.Core/Routing/InnerAttributeRoute.cs b/src/Microsoft.AspNet.Mvc.Core/Routing/InnerAttributeRoute.cs index 546270314f..e4dbbafa5d 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Routing/InnerAttributeRoute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Routing/InnerAttributeRoute.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Threading.Tasks; using Microsoft.AspNet.Mvc.Core; using Microsoft.AspNet.Mvc.Internal.Routing; +using Microsoft.AspNet.Mvc.Logging; using Microsoft.AspNet.Routing; using Microsoft.AspNet.Routing.Template; using Microsoft.Extensions.Logging; @@ -156,8 +157,7 @@ namespace Microsoft.AspNet.Mvc.Routing continue; } - _logger.LogVerbose( - "Request successfully matched the route with name '{RouteName}' and template '{RouteTemplate}'.", + _logger.MatchedRouteName( matchingEntry.RouteName, matchingEntry.RouteTemplate); diff --git a/src/Microsoft.AspNet.Mvc.Core/Logging/JsonResultLoggerExtensions.cs b/src/Microsoft.AspNet.Mvc.Formatters.Json/Logging/JsonResultLoggerExtensions.cs similarity index 83% rename from src/Microsoft.AspNet.Mvc.Core/Logging/JsonResultLoggerExtensions.cs rename to src/Microsoft.AspNet.Mvc.Formatters.Json/Logging/JsonResultLoggerExtensions.cs index d1c05988e5..095f1ccaa9 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Logging/JsonResultLoggerExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.Formatters.Json/Logging/JsonResultLoggerExtensions.cs @@ -6,9 +6,9 @@ using Microsoft.Extensions.Logging; namespace Microsoft.AspNet.Mvc.Logging { - public static class JsonResultLoggerExtensions + internal static class JsonResultLoggerExtensions { - private static Action _jsonResultExecuting; + private static readonly Action _jsonResultExecuting; static JsonResultLoggerExtensions() { diff --git a/src/Microsoft.AspNet.Mvc.TagHelpers/Internal/ModeMatchResult.cs b/src/Microsoft.AspNet.Mvc.TagHelpers/Internal/ModeMatchResult.cs index a92a0bda3e..c70f5d1443 100644 --- a/src/Microsoft.AspNet.Mvc.TagHelpers/Internal/ModeMatchResult.cs +++ b/src/Microsoft.AspNet.Mvc.TagHelpers/Internal/ModeMatchResult.cs @@ -4,7 +4,8 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Reflection; +using Microsoft.AspNet.Mvc.Logging; +using Microsoft.AspNet.Mvc.TagHelpers.Logging; using Microsoft.AspNet.Razor.TagHelpers; using Microsoft.Extensions.Logging; @@ -63,17 +64,15 @@ namespace Microsoft.AspNet.Mvc.TagHelpers.Internal match => match.PresentAttributes.Any( attribute => PartiallyMatchedAttributes.Contains( attribute, StringComparer.OrdinalIgnoreCase))); - - logger.LogWarning(new PartialModeMatchLogValues(uniqueId, viewPath, partialOnlyMatches)); + logger.TagHelperPartialMatches(uniqueId, viewPath, partialOnlyMatches); } if (logger.IsEnabled(LogLevel.Verbose) && !FullMatches.Any()) { - logger.LogVerbose( - "Skipping processing for tag helper '{TagHelper}' with id '{TagHelperId}'.", - tagHelper.GetType().GetTypeInfo().FullName, + logger.TagHelperSkippingProcessing( + tagHelper, uniqueId); } } } -} \ No newline at end of file +} diff --git a/src/Microsoft.AspNet.Mvc.TagHelpers/Internal/PartialModeMatchLogValuesOfT.cs b/src/Microsoft.AspNet.Mvc.TagHelpers/Internal/PartialModeMatchLogValuesOfT.cs deleted file mode 100644 index 9bd33a3a53..0000000000 --- a/src/Microsoft.AspNet.Mvc.TagHelpers/Internal/PartialModeMatchLogValuesOfT.cs +++ /dev/null @@ -1,63 +0,0 @@ -// 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 System.Collections.Generic; -using System.Linq; -using Microsoft.Extensions.Logging; - -namespace Microsoft.AspNet.Mvc.TagHelpers.Internal -{ - /// - /// Log values for instances that opt out of - /// processing due to missing attributes for one of several possible modes. - /// - public class PartialModeMatchLogValues : ILogValues - { - private readonly string _uniqueId; - private readonly string _viewPath; - private readonly IEnumerable> _partialMatches; - - /// - /// Creates a new . - /// - /// The unique ID of the HTML element this message applies to. - /// The path to the view. - /// The set of modes with partial required attributes. - public PartialModeMatchLogValues( - string uniqueId, - string viewPath, - IEnumerable> partialMatches) - { - if (partialMatches == null) - { - throw new ArgumentNullException(nameof(partialMatches)); - } - - _uniqueId = uniqueId; - _viewPath = viewPath; - _partialMatches = partialMatches; - } - - public override string ToString() - { - var newLine = Environment.NewLine; - return string.Format( - $"Tag Helper with ID '{_uniqueId}' in view '{_viewPath}' had partial matches " + - $"while determining mode:{newLine}\t{{0}}", - string.Join($"{newLine}\t", _partialMatches.Select(partial => - string.Format($"Mode '{partial.Mode}' missing attributes:{newLine}\t\t{{0}} ", - string.Join($"{newLine}\t\t", partial.MissingAttributes))))); - } - - public IEnumerable> GetValues() - { - yield return new KeyValuePair( - "Message", - "Tag helper had partial matches while determining mode."); - yield return new KeyValuePair("UniqueId", _uniqueId); - yield return new KeyValuePair("ViewPath", _viewPath); - yield return new KeyValuePair("PartialMatches", _partialMatches); - } - } -} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.TagHelpers/Logging/ModeMatchResultLoggerExtensions.cs b/src/Microsoft.AspNet.Mvc.TagHelpers/Logging/ModeMatchResultLoggerExtensions.cs new file mode 100644 index 0000000000..d1cbdf7d7e --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.TagHelpers/Logging/ModeMatchResultLoggerExtensions.cs @@ -0,0 +1,103 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Microsoft.AspNet.Mvc.TagHelpers.Internal; +using Microsoft.AspNet.Razor.TagHelpers; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNet.Mvc.TagHelpers.Logging +{ + internal static class ModeMatchResultLoggerExtensions + { + private static readonly Action _skippingProcessing; + + static ModeMatchResultLoggerExtensions() + { + _skippingProcessing = LoggerMessage.Define( + LogLevel.Verbose, + 1, + "Skipping processing for tag helper '{TagHelper}' with id '{TagHelperId}'."); + } + + public static void TagHelperPartialMatches( + this ILogger logger, + string uniqueId, + string viewPath, + IEnumerable> partialMatches) + { + var logValues = new PartialModeMatchLogValues( + uniqueId, + viewPath, + partialMatches); + + logger.LogWarning(logValues); + } + + public static void TagHelperSkippingProcessing( + this ILogger logger, + ITagHelper tagHelper, + string uniqueId) + { + _skippingProcessing( + logger, + tagHelper, + uniqueId, + null); + } + + /// + /// Log values for instances that opt out + /// of processing due to missing attributes for one of several possible modes. + /// + private class PartialModeMatchLogValues : ILogValues + { + private readonly IEnumerable> _partialMatches; + private readonly string _uniqueId; + private readonly string _viewPath; + + /// + /// Creates a new . + /// + /// + /// The unique ID of the HTML element this message applies to. + /// + /// The path to the view. + /// The set of modes with partial required attributes. + public PartialModeMatchLogValues( + string uniqueId, + string viewPath, + IEnumerable> partialMatches) + { + if (partialMatches == null) + { + throw new ArgumentNullException(nameof(partialMatches)); + } + + _uniqueId = uniqueId; + _viewPath = viewPath; + _partialMatches = partialMatches; + } + + public IEnumerable> GetValues() + { + yield return new KeyValuePair( + "Message", + "Tag helper had partial matches while determining mode."); + yield return new KeyValuePair("UniqueId", _uniqueId); + yield return new KeyValuePair("ViewPath", _viewPath); + yield return new KeyValuePair("PartialMatches", _partialMatches); + } + + public override string ToString() + { + var newLine = Environment.NewLine; + return string.Format( + $"Tag Helper with ID '{_uniqueId}' in view '{_viewPath}' had partial matches " + + $"while determining mode:{newLine}\t{{0}}", + string.Join($"{newLine}\t", _partialMatches.Select(partial => + string.Format($"Mode '{partial.Mode}' missing attributes:{newLine}\t\t{{0}} ", + string.Join($"{newLine}\t\t", partial.MissingAttributes))))); + } + } + } +} diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/Logging/PartialViewResultExecutorLoggerExtensions.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/Logging/PartialViewResultExecutorLoggerExtensions.cs index 411a674b86..27ac11c921 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/Logging/PartialViewResultExecutorLoggerExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/Logging/PartialViewResultExecutorLoggerExtensions.cs @@ -2,14 +2,17 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using Microsoft.AspNet.Mvc.ViewEngines; using Microsoft.Extensions.Logging; -namespace Microsoft.AspNet.Mvc.Logging +namespace Microsoft.AspNet.Mvc.ViewFeatures.Logging { - public static class PartialViewResultExecutorLoggerExtensions + internal static class PartialViewResultExecutorLoggerExtensions { - private static Action _partialViewResultExecuting; + private static readonly Action _partialViewFound; + private static readonly Action, Exception> _partialViewNotFound; + private static readonly Action _partialViewResultExecuting; static PartialViewResultExecutorLoggerExtensions() { @@ -17,6 +20,29 @@ namespace Microsoft.AspNet.Mvc.Logging LogLevel.Information, 1, "Executing PartialViewResult, running view at path {Path}."); + _partialViewFound = LoggerMessage.Define( + LogLevel.Verbose, + 2, + "The partial view '{PartialViewName}' was found."); + _partialViewNotFound = LoggerMessage.Define>( + LogLevel.Error, + 3, + "The partial view '{PartialViewName}' was not found. Searched locations: {SearchedViewLocations}"); + } + + public static void PartialViewFound( + this ILogger logger, + string partialViewName) + { + _partialViewFound(logger, partialViewName, null); + } + + public static void PartialViewNotFound( + this ILogger logger, + string partialViewName, + IEnumerable searchedLocations) + { + _partialViewNotFound(logger, partialViewName, searchedLocations, null); } public static void PartialViewResultExecuting(this ILogger logger, IView view) diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/PartialViewResultExecutor.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/PartialViewResultExecutor.cs index b2ac16d64f..99e23f11cb 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/PartialViewResultExecutor.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/PartialViewResultExecutor.cs @@ -8,6 +8,7 @@ using Microsoft.AspNet.Mvc.Diagnostics; using Microsoft.AspNet.Mvc.Infrastructure; using Microsoft.AspNet.Mvc.Logging; using Microsoft.AspNet.Mvc.ViewEngines; +using Microsoft.AspNet.Mvc.ViewFeatures.Logging; using Microsoft.Extensions.Logging; using Microsoft.Extensions.OptionsModel; @@ -73,16 +74,13 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures { DiagnosticSource.ViewFound(actionContext, true, viewResult, viewName, result.View); - Logger.LogVerbose("The partial view '{PartialViewName}' was found.", viewName); + Logger.PartialViewFound(viewName); } else { DiagnosticSource.ViewNotFound(actionContext, true, viewResult, viewName, result.SearchedLocations); - Logger.LogError( - "The partial view '{PartialViewName}' was not found. Searched locations: {SearchedViewLocations}", - viewName, - result.SearchedLocations); + Logger.PartialViewNotFound(viewName, result.SearchedLocations); } return result; diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/ViewResultExecutor.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/ViewResultExecutor.cs index 9f96afe837..9020f513c2 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/ViewResultExecutor.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/ViewResultExecutor.cs @@ -7,6 +7,7 @@ using System.Threading.Tasks; using Microsoft.AspNet.Mvc.Infrastructure; using Microsoft.AspNet.Mvc.Logging; using Microsoft.AspNet.Mvc.ViewEngines; +using Microsoft.AspNet.Mvc.ViewFeatures.Logging; using Microsoft.Extensions.Logging; using Microsoft.Extensions.OptionsModel; @@ -84,7 +85,7 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures }); } - Logger.LogVerbose("The view '{ViewName}' was found.", viewName); + Logger.PartialViewFound(viewName); } else { @@ -101,11 +102,7 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures searchedLocations = result.SearchedLocations }); } - - Logger.LogError( - "The view '{ViewName}' was not found. Searched locations: {SearchedViewLocations}", - viewName, - result.SearchedLocations); + Logger.PartialViewNotFound(viewName, result.SearchedLocations); } return result; @@ -135,7 +132,6 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures throw new ArgumentNullException(nameof(viewResult)); } - Logger.ViewResultExecuting(view); return ExecuteAsync( diff --git a/test/Microsoft.AspNet.Mvc.Razor.Test/RazorTextWriterTest.cs b/test/Microsoft.AspNet.Mvc.Razor.Test/RazorTextWriterTest.cs index 48ec1d6c58..76059b7696 100644 --- a/test/Microsoft.AspNet.Mvc.Razor.Test/RazorTextWriterTest.cs +++ b/test/Microsoft.AspNet.Mvc.Razor.Test/RazorTextWriterTest.cs @@ -330,8 +330,7 @@ namespace Microsoft.AspNet.Mvc.Razor.Test // Arrange var source = new RazorTextWriter(TextWriter.Null, Encoding.UTF8, new CommonTestEncoder()); var target = new StringWriter(); - var expected = @"Hello world -abc"; + var expected = "Hello world" + Environment.NewLine + "abc"; // Act source.WriteLine("Hello world"); @@ -390,8 +389,7 @@ abc"; // Arrange var source = new RazorTextWriter(TextWriter.Null, Encoding.UTF8, new CommonTestEncoder()); var target = new StringWriter(); - var expected = @"Hello world -"; + var expected = "Hello world" + Environment.NewLine; // Act source.Write("Hello ");