From f49cbd1b252b1f2084fef5673a352960c46c68a4 Mon Sep 17 00:00:00 2001 From: Jass Bagga Date: Fri, 29 Sep 2017 17:50:45 -0700 Subject: [PATCH] Selectors initialization and DispatcherBase logging (#451) --- .../Endpoint.cs | 2 +- .../AmbiguousEndpointException.cs | 25 ++++ .../DispatcherBase.cs | 66 +++++++++- .../DispatcherLoggerExtensions.cs | 44 +++++++ .../EndpointSelector.cs | 4 + .../Microsoft.AspNetCore.Dispatcher.csproj | 1 + .../Properties/Resources.Designer.cs | 44 +++++++ .../Resources.resx | 124 ++++++++++++++++++ .../Dispatcher/TreeDispatcher.cs | 31 +---- 9 files changed, 311 insertions(+), 30 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Dispatcher/AmbiguousEndpointException.cs create mode 100644 src/Microsoft.AspNetCore.Dispatcher/DispatcherLoggerExtensions.cs create mode 100644 src/Microsoft.AspNetCore.Dispatcher/Properties/Resources.Designer.cs create mode 100644 src/Microsoft.AspNetCore.Dispatcher/Resources.resx diff --git a/src/Microsoft.AspNetCore.Dispatcher.Abstractions/Endpoint.cs b/src/Microsoft.AspNetCore.Dispatcher.Abstractions/Endpoint.cs index a621c76ee3..ed3d96df58 100644 --- a/src/Microsoft.AspNetCore.Dispatcher.Abstractions/Endpoint.cs +++ b/src/Microsoft.AspNetCore.Dispatcher.Abstractions/Endpoint.cs @@ -6,7 +6,7 @@ using System.Diagnostics; namespace Microsoft.AspNetCore.Dispatcher { - [DebuggerDisplay("{DisplayName,nq}")] + [DebuggerDisplay("{GetType().Name} - '{DisplayName,nq}'")] public abstract class Endpoint { public abstract string DisplayName { get; } diff --git a/src/Microsoft.AspNetCore.Dispatcher/AmbiguousEndpointException.cs b/src/Microsoft.AspNetCore.Dispatcher/AmbiguousEndpointException.cs new file mode 100644 index 0000000000..4f54f46cb4 --- /dev/null +++ b/src/Microsoft.AspNetCore.Dispatcher/AmbiguousEndpointException.cs @@ -0,0 +1,25 @@ +// 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.Runtime.Serialization; + +namespace Microsoft.AspNetCore.Dispatcher +{ + /// + /// An exception which indicates multiple matches in endpoint selection. + /// + [Serializable] + public class AmbiguousEndpointException : Exception + { + public AmbiguousEndpointException(string message) + : base(message) + { + } + + protected AmbiguousEndpointException(SerializationInfo info, StreamingContext context) + : base(info, context) + { + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Dispatcher/DispatcherBase.cs b/src/Microsoft.AspNetCore.Dispatcher/DispatcherBase.cs index 5d3ecb2343..87617e04fe 100644 --- a/src/Microsoft.AspNetCore.Dispatcher/DispatcherBase.cs +++ b/src/Microsoft.AspNetCore.Dispatcher/DispatcherBase.cs @@ -4,9 +4,12 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.FileProviders; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Primitives; namespace Microsoft.AspNetCore.Dispatcher @@ -17,6 +20,21 @@ namespace Microsoft.AspNetCore.Dispatcher private List _endpoints; private List _endpointSelectors; + private object _initialize; + private bool _selectorsInitialized; + private readonly Func _initializer; + private object _lock; + + private bool _servicesInitialized; + + public DispatcherBase() + { + _lock = new object(); + _initializer = InitializeSelectors; + } + + protected ILogger Logger { get; private set; } + public virtual IList
Addresses { get @@ -81,6 +99,8 @@ namespace Microsoft.AspNetCore.Dispatcher throw new ArgumentNullException(nameof(httpContext)); } + EnsureServicesInitialized(httpContext); + var feature = httpContext.Features.Get(); if (await TryMatchAsync(httpContext)) { @@ -117,20 +137,64 @@ namespace Microsoft.AspNetCore.Dispatcher throw new ArgumentNullException(nameof(selectors)); } + LazyInitializer.EnsureInitialized(ref _initialize, ref _selectorsInitialized, ref _lock, _initializer); + var selectorContext = new EndpointSelectorContext(httpContext, endpoints.ToList(), selectors.ToList()); await selectorContext.InvokeNextAsync(); switch (selectorContext.Endpoints.Count) { case 0: + Logger.NoEndpointsMatched(httpContext.Request.Path); return null; case 1: + Logger.EndpointMatched(selectorContext.Endpoints[0].DisplayName); return selectorContext.Endpoints[0]; default: - throw new InvalidOperationException("Ambiguous bro!"); + var endpointNames = string.Join( + Environment.NewLine, + selectorContext.Endpoints.Select(a => a.DisplayName)); + Logger.AmbiguousEndpoints(endpointNames); + + var message = Resources.FormatAmbiguousEndpoints( + Environment.NewLine, + endpointNames); + + throw new AmbiguousEndpointException(message); + } + } + + private object InitializeSelectors() + { + foreach (var selector in Selectors) + { + selector.Initialize(this); + } + + return null; + } + + protected void EnsureServicesInitialized(HttpContext httpContext) + { + if (Volatile.Read(ref _servicesInitialized)) + { + return; + } + + EnsureServicesInitializedSlow(httpContext); + } + + private void EnsureServicesInitializedSlow(HttpContext httpContext) + { + lock (_lock) + { + if (!Volatile.Read(ref _servicesInitialized)) + { + Logger = httpContext.RequestServices.GetRequiredService().CreateLogger(GetType()); + } } } } diff --git a/src/Microsoft.AspNetCore.Dispatcher/DispatcherLoggerExtensions.cs b/src/Microsoft.AspNetCore.Dispatcher/DispatcherLoggerExtensions.cs new file mode 100644 index 0000000000..7381430f4c --- /dev/null +++ b/src/Microsoft.AspNetCore.Dispatcher/DispatcherLoggerExtensions.cs @@ -0,0 +1,44 @@ +// 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.AspNetCore.Http; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNetCore.Dispatcher +{ + internal static class DispatcherLoggerExtensions + { + // Too many matches + private static readonly Action _ambiguousEndpoints = LoggerMessage.Define( + LogLevel.Error, + new EventId(1, "AmbiguousEndpoints"), + "Request matched multiple endpoints resulting in ambiguity. Matching endpoints: {AmbiguousEndpoints}"); + + // Unique match found + private static readonly Action _endpointMatched = LoggerMessage.Define( + LogLevel.Information, + new EventId(1, "EndpointMatched"), + "Request matched endpoint: {endpointName}"); + + private static readonly Action _noEndpointsMatched = LoggerMessage.Define( + LogLevel.Debug, + new EventId(2, "NoEndpointsMatched"), + "No endpoints matched the current request path: {PathString}"); + + public static void AmbiguousEndpoints(this ILogger logger, string ambiguousEndpoints) + { + _ambiguousEndpoints(logger, ambiguousEndpoints, null); + } + + public static void EndpointMatched(this ILogger logger, string endpointName) + { + _endpointMatched(logger, endpointName ?? "Unnamed endpoint", null); + } + + public static void NoEndpointsMatched(this ILogger logger, PathString pathString) + { + _noEndpointsMatched(logger, pathString, null); + } + } +} diff --git a/src/Microsoft.AspNetCore.Dispatcher/EndpointSelector.cs b/src/Microsoft.AspNetCore.Dispatcher/EndpointSelector.cs index d98bd9bf78..4d1f540731 100644 --- a/src/Microsoft.AspNetCore.Dispatcher/EndpointSelector.cs +++ b/src/Microsoft.AspNetCore.Dispatcher/EndpointSelector.cs @@ -8,5 +8,9 @@ namespace Microsoft.AspNetCore.Dispatcher public abstract class EndpointSelector { public abstract Task SelectAsync(EndpointSelectorContext context); + + public virtual void Initialize(IEndpointCollectionProvider endpointProvider) + { + } } } diff --git a/src/Microsoft.AspNetCore.Dispatcher/Microsoft.AspNetCore.Dispatcher.csproj b/src/Microsoft.AspNetCore.Dispatcher/Microsoft.AspNetCore.Dispatcher.csproj index 31a0a60990..5c27ac5cb0 100644 --- a/src/Microsoft.AspNetCore.Dispatcher/Microsoft.AspNetCore.Dispatcher.csproj +++ b/src/Microsoft.AspNetCore.Dispatcher/Microsoft.AspNetCore.Dispatcher.csproj @@ -15,6 +15,7 @@ + diff --git a/src/Microsoft.AspNetCore.Dispatcher/Properties/Resources.Designer.cs b/src/Microsoft.AspNetCore.Dispatcher/Properties/Resources.Designer.cs new file mode 100644 index 0000000000..2a15f90c99 --- /dev/null +++ b/src/Microsoft.AspNetCore.Dispatcher/Properties/Resources.Designer.cs @@ -0,0 +1,44 @@ +// +namespace Microsoft.AspNetCore.Dispatcher +{ + using System.Globalization; + using System.Reflection; + using System.Resources; + + internal static class Resources + { + private static readonly ResourceManager _resourceManager + = new ResourceManager("Microsoft.AspNetCore.Dispatcher.Resources", typeof(Resources).GetTypeInfo().Assembly); + + /// + /// Multiple endpoints matched. The following endpoints matched the request:{0}{0}{1} + /// + internal static string AmbiguousEndpoints + { + get => GetString("AmbiguousEndpoints"); + } + + /// + /// Multiple endpoints matched. The following endpoints matched the request:{0}{0}{1} + /// + internal static string FormatAmbiguousEndpoints(object p0, object p1) + => string.Format(CultureInfo.CurrentCulture, GetString("AmbiguousEndpoints"), p0, p1); + + private static string GetString(string name, params string[] formatterNames) + { + var value = _resourceManager.GetString(name); + + System.Diagnostics.Debug.Assert(value != null); + + if (formatterNames != null) + { + for (var i = 0; i < formatterNames.Length; i++) + { + value = value.Replace("{" + formatterNames[i] + "}", "{" + i + "}"); + } + } + + return value; + } + } +} diff --git a/src/Microsoft.AspNetCore.Dispatcher/Resources.resx b/src/Microsoft.AspNetCore.Dispatcher/Resources.resx new file mode 100644 index 0000000000..f7aa8ef240 --- /dev/null +++ b/src/Microsoft.AspNetCore.Dispatcher/Resources.resx @@ -0,0 +1,124 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + text/microsoft-resx + + + 2.0 + + + System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + Multiple endpoints matched. The following endpoints matched the request:{0}{0}{1} + 0 is the newline - 1 is a newline separate list of action display names + + \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Routing/Dispatcher/TreeDispatcher.cs b/src/Microsoft.AspNetCore.Routing/Dispatcher/TreeDispatcher.cs index b2ebd951d0..cdad000ff9 100644 --- a/src/Microsoft.AspNetCore.Routing/Dispatcher/TreeDispatcher.cs +++ b/src/Microsoft.AspNetCore.Routing/Dispatcher/TreeDispatcher.cs @@ -14,7 +14,6 @@ using Microsoft.AspNetCore.Routing.Internal; using Microsoft.AspNetCore.Routing.Logging; using Microsoft.AspNetCore.Routing.Template; using Microsoft.AspNetCore.Routing.Tree; -using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Internal; using Microsoft.Extensions.Logging; @@ -23,14 +22,11 @@ namespace Microsoft.AspNetCore.Routing.Dispatcher public class TreeDispatcher : DispatcherBase { private bool _dataInitialized; - private bool _servicesInitialized; private object _lock; private Cache _cache; private readonly Func _initializer; - private ILogger _logger; - public TreeDispatcher() { _lock = new object(); @@ -66,14 +62,14 @@ namespace Microsoft.AspNetCore.Routing.Dispatcher { var entry = item.Entry; var matcher = item.TemplateMatcher; - + values.Clear(); if (!matcher.TryMatch(httpContext.Request.Path, values)) { continue; } - _logger.MatchedRoute(entry.RouteName, entry.RouteTemplate.TemplateText); + Logger.MatchedRoute(entry.RouteName, entry.RouteTemplate.TemplateText); if (!MatchConstraints(httpContext, values, entry.Constraints)) { @@ -102,7 +98,7 @@ namespace Microsoft.AspNetCore.Routing.Dispatcher object value; values.TryGetValue(kvp.Key, out value); - _logger.RouteValueDoesNotMatchConstraint(value, kvp.Key, kvp.Value); + Logger.RouteValueDoesNotMatchConstraint(value, kvp.Key, kvp.Value); return false; } } @@ -111,27 +107,6 @@ namespace Microsoft.AspNetCore.Routing.Dispatcher return true; } - private void EnsureServicesInitialized(HttpContext httpContext) - { - if (Volatile.Read(ref _servicesInitialized)) - { - return; - } - - EnsureServicesInitializedSlow(httpContext); - } - - private void EnsureServicesInitializedSlow(HttpContext httpContext) - { - lock (_lock) - { - if (!Volatile.Read(ref _servicesInitialized)) - { - _logger = httpContext.RequestServices.GetRequiredService>(); - } - } - } - private Cache CreateCache() { var endpoints = GetEndpoints();