diff --git a/src/Microsoft.AspNet.Routing/Logging/RouteCollectionRouteAsyncValues.cs b/src/Microsoft.AspNet.Routing/Logging/RouteCollectionRouteAsyncValues.cs deleted file mode 100644 index da340786..00000000 --- a/src/Microsoft.AspNet.Routing/Logging/RouteCollectionRouteAsyncValues.cs +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Collections.Generic; -using System.Text; - -namespace Microsoft.AspNet.Routing.Logging -{ - /// - /// Describes the state of - /// . - /// - public class RouteCollectionRouteAsyncValues - { - /// - /// The name of the state. - /// - public string Name - { - get - { - return "RouteCollection.RouteAsync"; - } - } - - /// - /// The available routes. - /// - public IList Routes { get; set; } - - /// - /// True if the request is handled. - /// - public bool Handled { get; set; } - - /// - /// A summary of the data for display. - /// - public string Summary - { - get - { - var builder = new StringBuilder(); - builder.AppendLine(Name); - builder.Append("\tRoutes: "); - StringBuilderHelpers.Append(builder, Routes); - builder.AppendLine(); - builder.Append("\tHandled? "); - builder.Append(Handled); - return builder.ToString(); - } - } - - /// - public override string ToString() - { - return Summary; - } - } -} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Routing/Logging/RouteConstraintMatcherMatchValues.cs b/src/Microsoft.AspNet.Routing/Logging/RouteConstraintMatcherMatchValues.cs deleted file mode 100644 index fa5b8e37..00000000 --- a/src/Microsoft.AspNet.Routing/Logging/RouteConstraintMatcherMatchValues.cs +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Text; - -namespace Microsoft.AspNet.Routing.Logging -{ - /// - /// Describes the state of . - /// - public class RouteConstraintMatcherMatchValues - { - /// - /// The name of the state. - /// - public string Name - { - get - { - return "RouteConstraintMatcher.Match"; - } - } - - /// - /// The key of the constraint. - /// - public string ConstraintKey { get; set; } - - /// - /// The constraint. - /// - public IRouteConstraint Constraint { get; set; } - - /// - /// True if the matched. - /// - public bool Matched { get; set; } - - /// - /// A summary of the data for display. - /// - public string Summary - { - get - { - var builder = new StringBuilder(); - builder.AppendLine(Name); - builder.Append("\tConstraint key: "); - builder.AppendLine(ConstraintKey); - builder.Append("\tConstraint: "); - builder.Append(Constraint); - builder.AppendLine(); - builder.Append("\tMatched? "); - builder.Append(Matched); - return builder.ToString(); - } - } - - /// - public override string ToString() - { - return Summary; - } - } -} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Routing/Logging/RouterMiddlewareInvokeValues.cs b/src/Microsoft.AspNet.Routing/Logging/RouterMiddlewareInvokeValues.cs deleted file mode 100644 index d99b834f..00000000 --- a/src/Microsoft.AspNet.Routing/Logging/RouterMiddlewareInvokeValues.cs +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Text; - -namespace Microsoft.AspNet.Routing.Logging -{ - /// - /// Describes the state of - /// . - /// - public class RouterMiddlewareInvokeValues - { - /// - /// The name of the state. - /// - public string Name - { - get - { - return "RouterMiddleware.Invoke"; - } - } - - /// - /// True if the request is handled. - /// - public bool Handled { get; set; } - - /// - /// A summary of the data for display. - /// - public string Summary - { - get - { - var builder = new StringBuilder(); - builder.AppendLine(Name); - builder.Append("\tHandled? "); - builder.Append(Handled); - return builder.ToString(); - } - } - - /// - public override string ToString() - { - return Summary; - } - } -} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Routing/Logging/TemplateRouteRouteAsyncValues.cs b/src/Microsoft.AspNet.Routing/Logging/TemplateRouteRouteAsyncValues.cs deleted file mode 100644 index 18f83c6b..00000000 --- a/src/Microsoft.AspNet.Routing/Logging/TemplateRouteRouteAsyncValues.cs +++ /dev/null @@ -1,122 +0,0 @@ -// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Collections.Generic; -using System.Text; - -namespace Microsoft.AspNet.Routing.Logging -{ - /// - /// Describes the state of - /// . - /// - public class TemplateRouteRouteAsyncValues - { - /// - /// The name of the state. - /// - public string Name - { - get - { - return "TemplateRoute.RouteAsync"; - } - } - - /// - /// The target. - /// - public IRouter Target { get; set; } - - /// - /// The template. - /// - public string Template { get; set; } - - /// - /// The request path. - /// - public string RequestPath { get; set; } - - /// - /// The values produced by default. - /// - public IReadOnlyDictionary DefaultValues { get; set; } - - /// - /// The values produced from the request. - /// - public IDictionary ProducedValues { get; set; } - - /// - /// The constraints matched on the produced values. - /// - public IReadOnlyDictionary Constraints { get; set; } - - /// - /// True if the matched. - /// - public bool MatchedTemplate { get; set; } - - /// - /// True if the matched. - /// - public bool MatchedConstraints { get; set; } - - /// - /// True if this route matched. - /// - public bool Matched { get; set; } - - /// - /// True if the request is handled. - /// - public bool Handled { get; set; } - - /// - /// A summary of the data for display. - /// - public string Summary - { - get - { - var builder = new StringBuilder(); - builder.AppendLine(Name); - builder.Append("\tTarget: "); - builder.Append(Target); - builder.AppendLine(); - builder.Append("\tTemplate: "); - builder.AppendLine(Template); - builder.Append("\tRequest path: "); - builder.AppendLine(RequestPath); - builder.Append("\tDefault values: "); - StringBuilderHelpers.Append(builder, DefaultValues); - builder.AppendLine(); - builder.Append("\tProduced values: "); - StringBuilderHelpers.Append(builder, ProducedValues); - builder.AppendLine(); - builder.Append("\tConstraints: "); - StringBuilderHelpers.Append(builder, Constraints); - builder.AppendLine(); - builder.Append("\tMatched template? "); - builder.Append(MatchedTemplate); - builder.AppendLine(); - builder.Append("\tMatched constraints? "); - builder.Append(MatchedConstraints); - builder.AppendLine(); - builder.Append("\tMatched? "); - builder.Append(Matched); - builder.AppendLine(); - builder.Append("\tHandled? "); - builder.Append(Handled); - return builder.ToString(); - } - } - - /// - public override string ToString() - { - return Summary; - } - } -} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Routing/RouteCollection.cs b/src/Microsoft.AspNet.Routing/RouteCollection.cs index 82eb3482..eb1bb478 100644 --- a/src/Microsoft.AspNet.Routing/RouteCollection.cs +++ b/src/Microsoft.AspNet.Routing/RouteCollection.cs @@ -22,7 +22,6 @@ public class RouteCollection : IRouteCollection private readonly Dictionary _namedRoutes = new Dictionary(StringComparer.OrdinalIgnoreCase); - private ILogger _logger; private RouteOptions _options; public IRouter this[int index] @@ -55,44 +54,31 @@ public void Add([NotNull] IRouter router) public async virtual Task RouteAsync(RouteContext context) { - EnsureLogger(context.HttpContext); - using (_logger.BeginScope("RouteCollection.RouteAsync")) + for (var i = 0; i < Count; i++) { - for (var i = 0; i < Count; i++) - { - var route = this[i]; + var route = this[i]; - var oldRouteData = context.RouteData; + var oldRouteData = context.RouteData; - var newRouteData = new RouteData(oldRouteData); - newRouteData.Routers.Add(route); + var newRouteData = new RouteData(oldRouteData); + newRouteData.Routers.Add(route); - try - { - context.RouteData = newRouteData; + try + { + context.RouteData = newRouteData; - await route.RouteAsync(context); - if (context.IsHandled) - { - break; - } - } - finally + await route.RouteAsync(context); + if (context.IsHandled) { - if (!context.IsHandled) - { - context.RouteData = oldRouteData; - } + break; } } - - if (_logger.IsEnabled(LogLevel.Verbose)) + finally { - _logger.WriteValues(new RouteCollectionRouteAsyncValues() + if (!context.IsHandled) { - Handled = context.IsHandled, - Routes = _routes - }); + context.RouteData = oldRouteData; + } } } } @@ -241,15 +227,6 @@ private PathString NormalizeVirtualPath(PathString path) return path; } - private void EnsureLogger(HttpContext context) - { - if (_logger == null) - { - var factory = context.RequestServices.GetRequiredService(); - _logger = factory.CreateLogger(); - } - } - private void EnsureOptions(HttpContext context) { if (_options == null) diff --git a/src/Microsoft.AspNet.Routing/RouteConstraintMatcher.cs b/src/Microsoft.AspNet.Routing/RouteConstraintMatcher.cs index 6acc14a2..34c348fe 100644 --- a/src/Microsoft.AspNet.Routing/RouteConstraintMatcher.cs +++ b/src/Microsoft.AspNet.Routing/RouteConstraintMatcher.cs @@ -3,8 +3,6 @@ using System.Collections.Generic; using Microsoft.AspNet.Http; -using Microsoft.AspNet.Routing.Logging; -using Microsoft.AspNet.Routing.Logging.Internal; using Microsoft.Framework.Internal; using Microsoft.Framework.Logging; @@ -29,30 +27,21 @@ public static bool Match(IReadOnlyDictionary constrain var constraint = kvp.Value; if (!constraint.Match(httpContext, route, kvp.Key, routeValues, routeDirection)) { - if (routeDirection.Equals(RouteDirection.IncomingRequest) - && logger.IsEnabled(LogLevel.Verbose)) + if (routeDirection.Equals(RouteDirection.IncomingRequest)) { - logger.WriteValues(new RouteConstraintMatcherMatchValues() - { - ConstraintKey = kvp.Key, - Constraint = kvp.Value, - Matched = false - }); + object routeValue; + routeValues.TryGetValue(kvp.Key, out routeValue); + + logger.LogVerbose( + "Route value '{RouteValue}' with key '{RouteKey}' did not match " + + "the constraint '{RouteConstraint}'.", + routeValue, + kvp.Key, + kvp.Value); } return false; } - - if (routeDirection.Equals(RouteDirection.IncomingRequest) - && logger.IsEnabled(LogLevel.Verbose)) - { - logger.WriteValues(new RouteConstraintMatcherMatchValues() - { - ConstraintKey = kvp.Key, - Constraint = kvp.Value, - Matched = true - }); - } } return true; diff --git a/src/Microsoft.AspNet.Routing/RouterMiddleware.cs b/src/Microsoft.AspNet.Routing/RouterMiddleware.cs index 84d0f3d8..811c3d6d 100644 --- a/src/Microsoft.AspNet.Routing/RouterMiddleware.cs +++ b/src/Microsoft.AspNet.Routing/RouterMiddleware.cs @@ -30,22 +30,16 @@ public RouterMiddleware( public async Task Invoke(HttpContext httpContext) { - using (_logger.BeginScope("RouterMiddleware.Invoke")) - { - var context = new RouteContext(httpContext); - context.RouteData.Routers.Add(_router); + var context = new RouteContext(httpContext); + context.RouteData.Routers.Add(_router); - await _router.RouteAsync(context); + await _router.RouteAsync(context); - if (_logger.IsEnabled(LogLevel.Verbose)) - { - _logger.WriteValues(new RouterMiddlewareInvokeValues() { Handled = context.IsHandled }); - } + if (!context.IsHandled) + { + _logger.LogVerbose("Request did not match any routes."); - if (!context.IsHandled) - { - await _next.Invoke(httpContext); - } + await _next.Invoke(httpContext); } } } diff --git a/src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs b/src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs index a65ef61b..8b5b9af9 100644 --- a/src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs +++ b/src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs @@ -5,8 +5,6 @@ using System.Collections.Generic; using System.Threading.Tasks; using Microsoft.AspNet.Http; -using Microsoft.AspNet.Routing.Logging; -using Microsoft.AspNet.Routing.Logging.Internal; using Microsoft.Framework.DependencyInjection; using Microsoft.Framework.Internal; using Microsoft.Framework.Logging; @@ -99,84 +97,63 @@ public IReadOnlyDictionary Constraints public async virtual Task RouteAsync([NotNull] RouteContext context) { EnsureLoggers(context.HttpContext); - using (_logger.BeginScope("TemplateRoute.RouteAsync")) - { - var requestPath = context.HttpContext.Request.Path.Value; - if (!string.IsNullOrEmpty(requestPath) && requestPath[0] == '/') - { - requestPath = requestPath.Substring(1); - } + var requestPath = context.HttpContext.Request.Path.Value; - var values = _matcher.Match(requestPath); + if (!string.IsNullOrEmpty(requestPath) && requestPath[0] == '/') + { + requestPath = requestPath.Substring(1); + } - if (values == null) - { - if (_logger.IsEnabled(LogLevel.Verbose)) - { - _logger.WriteValues(CreateRouteAsyncValues( - requestPath, - context.RouteData.Values, - matchedValues: false, - matchedConstraints: false, - handled: context.IsHandled)); - } + var values = _matcher.Match(requestPath); - // If we got back a null value set, that means the URI did not match - return; - } + if (values == null) + { + // If we got back a null value set, that means the URI did not match - var oldRouteData = context.RouteData; + _logger.LogVerbose( + "Request did not match the route with name '{RouteName}' and template '{RouteTemplate}'.", + Name, + RouteTemplate); - var newRouteData = new RouteData(oldRouteData); - MergeValues(newRouteData.DataTokens, _dataTokens); - newRouteData.Routers.Add(_target); - MergeValues(newRouteData.Values, values); + return; + } - if (!RouteConstraintMatcher.Match( - Constraints, - newRouteData.Values, - context.HttpContext, - this, - RouteDirection.IncomingRequest, - _constraintLogger)) - { - if (_logger.IsEnabled(LogLevel.Verbose)) - { - _logger.WriteValues(CreateRouteAsyncValues( - requestPath, - newRouteData.Values, - matchedValues: true, - matchedConstraints: false, - handled: context.IsHandled)); - } + var oldRouteData = context.RouteData; - return; - } + var newRouteData = new RouteData(oldRouteData); + MergeValues(newRouteData.DataTokens, _dataTokens); + newRouteData.Routers.Add(_target); + MergeValues(newRouteData.Values, values); - try - { - context.RouteData = newRouteData; + if (!RouteConstraintMatcher.Match( + Constraints, + newRouteData.Values, + context.HttpContext, + this, + RouteDirection.IncomingRequest, + _constraintLogger)) + { + return; + } - await _target.RouteAsync(context); + _logger.LogInformation( + "Request successfully matched the route with name '{RouteName}' and template '{RouteTemplate}'.", + Name, + RouteTemplate); - if (_logger.IsEnabled(LogLevel.Verbose)) - { - _logger.WriteValues(CreateRouteAsyncValues( - requestPath, - newRouteData.Values, - matchedValues: true, - matchedConstraints: true, - handled: context.IsHandled)); - } - } - finally + try + { + context.RouteData = newRouteData; + + await _target.RouteAsync(context); + } + finally + { + // Restore the original values to prevent polluting the route data. + if (!context.IsHandled) { - // Restore the original values to prevent polluting the route data. - if (!context.IsHandled) - { - context.RouteData = oldRouteData; - } + context.RouteData = oldRouteData; } } } @@ -327,27 +304,6 @@ private static RouteValueDictionary GetDefaults( return result; } - private TemplateRouteRouteAsyncValues CreateRouteAsyncValues( - string requestPath, - IDictionary producedValues, - bool matchedValues, - bool matchedConstraints, - bool handled) - { - var values = new TemplateRouteRouteAsyncValues(); - values.Template = _routeTemplate; - values.RequestPath = requestPath; - values.DefaultValues = Defaults; - values.ProducedValues = producedValues; - values.Constraints = _constraints; - values.Target = _target; - values.MatchedTemplate = matchedValues; - values.MatchedConstraints = matchedConstraints; - values.Matched = matchedValues && matchedConstraints; - values.Handled = handled; - return values; - } - private static void MergeValues( IDictionary destination, IDictionary values) diff --git a/test/Microsoft.AspNet.Routing.Tests/ConstraintMatcherTest.cs b/test/Microsoft.AspNet.Routing.Tests/ConstraintMatcherTest.cs index 8247a9c9..814281b9 100644 --- a/test/Microsoft.AspNet.Routing.Tests/ConstraintMatcherTest.cs +++ b/test/Microsoft.AspNet.Routing.Tests/ConstraintMatcherTest.cs @@ -57,88 +57,18 @@ public void MatchFail_LogsCorrectData() {"a", new PassConstraint()}, {"b", new FailConstraint()} }; - var sink = SetUpMatch(constraints, true); + var sink = SetUpMatch(constraints, loggerEnabled: true); + var expectedMessage = "Route value 'value' with key 'b' did not match the constraint " + + $"'{typeof(FailConstraint).FullName}'."; // Assert - // There are no begin scopes called. Assert.Empty(sink.Scopes); - - // There are two records for WriteCore. - Assert.Equal(2, sink.Writes.Count); - - var write = sink.Writes[0]; - Assert.Equal(_name, write.LoggerName); - var values = Assert.IsType(write.State); - Assert.Equal("RouteConstraintMatcher.Match", values.Name); - Assert.Equal("a", values.ConstraintKey); - Assert.Equal(constraints["a"], values.Constraint); - Assert.Equal(true, values.Matched); - - write = sink.Writes[1]; - Assert.Equal(_name, write.LoggerName); - values = Assert.IsType(write.State); - Assert.Equal("RouteConstraintMatcher.Match", values.Name); - Assert.Equal("b", values.ConstraintKey); - Assert.Equal(constraints["b"], values.Constraint); - Assert.Equal(false, values.Matched); - } - - [Fact] - public void MatchFail_DisabledLoggerDoesNotLog() - { - // Arrange & Act - var constraints = new Dictionary - { - {"a", new PassConstraint()}, - {"b", new FailConstraint()} - }; - var sink = SetUpMatch(constraints, false); - - // Assert - // There are no begin scopes called. - Assert.Empty(sink.Scopes); - - // Logger is disabled so it should not write - Assert.Empty(sink.Writes); + Assert.Single(sink.Writes); + Assert.Equal(expectedMessage, sink.Writes[0].State?.ToString()); } [Fact] - public void MatchSuccess_LogsCorrectData() - { - // Arrange & Act - var constraints = new Dictionary - { - {"a", new PassConstraint()}, - {"b", new PassConstraint()} - }; - var sink = SetUpMatch(constraints, true); - - // Assert - // There are no begin scopes called. - Assert.Empty(sink.Scopes); - - // There are two records WriteCore. - Assert.Equal(2, sink.Writes.Count); - - var write = sink.Writes[0]; - Assert.Equal(_name, write.LoggerName); - var values = Assert.IsType(write.State); - Assert.Equal("RouteConstraintMatcher.Match", values.Name); - Assert.Equal("a", values.ConstraintKey); - Assert.Equal(constraints["a"], values.Constraint); - Assert.Equal(true, values.Matched); - - write = sink.Writes[1]; - Assert.Equal(_name, write.LoggerName); - values = Assert.IsType(write.State); - Assert.Equal("RouteConstraintMatcher.Match", values.Name); - Assert.Equal("b", values.ConstraintKey); - Assert.Equal(constraints["b"], values.Constraint); - Assert.Equal(true, values.Matched); - } - - [Fact] - public void MatchSuccess_DisabledLoggerDoesNotLog() + public void MatchSuccess_DoesNotLog() { // Arrange & Act var constraints = new Dictionary @@ -149,10 +79,7 @@ public void MatchSuccess_DisabledLoggerDoesNotLog() var sink = SetUpMatch(constraints, false); // Assert - // There are no begin scopes called. Assert.Empty(sink.Scopes); - - // Disabled Logger should not write Assert.Empty(sink.Writes); } @@ -193,7 +120,7 @@ public void ConstraintsGetTheRightKey() httpContext: new Mock().Object, route: new Mock().Object, routeDirection: RouteDirection.IncomingRequest, - logger: NullLogger.Instance)); + logger: NullLogger.Instance)); } [Fact] @@ -268,11 +195,11 @@ public void ReturnsTrueOnNullInput() logger: NullLogger.Instance)); } - private TestSink SetUpMatch(Dictionary constraints, bool enabled) + private TestSink SetUpMatch(Dictionary constraints, bool loggerEnabled) { // Arrange var sink = new TestSink(); - var logger = new TestLogger(_name, sink, enabled); + var logger = new TestLogger(_name, sink, loggerEnabled); var routeValueDictionary = new RouteValueDictionary(new { a = "value", b = "value" }); @@ -324,4 +251,4 @@ public bool Match(HttpContext httpContext, } } } -} +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.Routing.Tests/RouteCollectionTest.cs b/test/Microsoft.AspNet.Routing.Tests/RouteCollectionTest.cs index e723f467..354d2504 100644 --- a/test/Microsoft.AspNet.Routing.Tests/RouteCollectionTest.cs +++ b/test/Microsoft.AspNet.Routing.Tests/RouteCollectionTest.cs @@ -115,83 +115,6 @@ public void GetVirtualPath_ReturnsDataTokens(RouteValueDictionary dataTokens, st } } - [Fact] - public async Task RouteAsync_LogsCorrectValuesWhenHandled() - { - // Arrange & Act - var sink = await SetUp(enabled: true, handled: true); - - // Assert - Assert.Single(sink.Scopes); - var scope = sink.Scopes[0]; - Assert.Equal(typeof(RouteCollection).FullName, scope.LoggerName); - Assert.Equal("RouteCollection.RouteAsync", scope.Scope.ToString()); - - Assert.Single(sink.Writes); - - var write = sink.Writes[0]; - Assert.Equal(typeof(RouteCollection).FullName, write.LoggerName); - Assert.Equal("RouteCollection.RouteAsync", write.Scope.ToString()); - var values = Assert.IsType(write.State); - Assert.Equal("RouteCollection.RouteAsync", values.Name); - Assert.NotNull(values.Routes); - Assert.Equal(true, values.Handled); - } - - [Fact] - public async Task RouteAsync_DoesNotLogWhenDisabledAndHandled() - { - // Arrange & Act - var sink = await SetUp(enabled: false, handled: true); - - // Assert - Assert.Single(sink.Scopes); - var scope = sink.Scopes[0]; - Assert.Equal(typeof(RouteCollection).FullName, scope.LoggerName); - Assert.Equal("RouteCollection.RouteAsync", scope.Scope.ToString()); - - Assert.Empty(sink.Writes); - } - - [Fact] - public async Task RouteAsync_LogsCorrectValuesWhenNotHandled() - { - // Arrange & Act - var sink = await SetUp(enabled: true, handled: false); - - // Assert - Assert.Single(sink.Scopes); - var scope = sink.Scopes[0]; - Assert.Equal(typeof(RouteCollection).FullName, scope.LoggerName); - Assert.Equal("RouteCollection.RouteAsync", scope.Scope.ToString()); - - // There is a record for IsEnabled and one for WriteCore. - Assert.Single(sink.Writes); - - var write = sink.Writes[0]; - Assert.Equal(typeof(RouteCollection).FullName, write.LoggerName); - Assert.Equal("RouteCollection.RouteAsync", write.Scope.ToString()); - var values = Assert.IsType(write.State); - Assert.Equal("RouteCollection.RouteAsync", values.Name); - Assert.NotNull(values.Routes); - Assert.Equal(false, values.Handled); - } - - [Fact] - public async Task RouteAsync_DoesNotLogWhenDisabledAndNotHandled() - { - // Arrange & Act - var sink = await SetUp(enabled: false, handled: false); - - // Assert - Assert.Single(sink.Scopes); - var scope = sink.Scopes[0]; - Assert.Equal(typeof(RouteCollection).FullName, scope.LoggerName); - Assert.Equal("RouteCollection.RouteAsync", scope.Scope.ToString()); - - Assert.Empty(sink.Writes); - } - [Fact] public async Task RouteAsync_FirstMatches() { diff --git a/test/Microsoft.AspNet.Routing.Tests/RouterMiddlewareTest.cs b/test/Microsoft.AspNet.Routing.Tests/RouterMiddlewareTest.cs index 21d13e8b..d9d1e196 100644 --- a/test/Microsoft.AspNet.Routing.Tests/RouterMiddlewareTest.cs +++ b/test/Microsoft.AspNet.Routing.Tests/RouterMiddlewareTest.cs @@ -5,7 +5,6 @@ using System.Threading.Tasks; using Microsoft.AspNet.Builder; using Microsoft.AspNet.Http.Core; -using Microsoft.AspNet.Routing.Logging; using Microsoft.Framework.Logging.Testing; using Xunit; @@ -14,9 +13,10 @@ namespace Microsoft.AspNet.Routing public class RouterMiddlewareTest { [Fact] - public async void Invoke_LogsCorrectValuesWhenNotHandled() + public async void Invoke_LogsCorrectValues_WhenNotHandled() { // Arrange + var expectedMessage = "Request did not match any routes."; var isHandled = false; var sink = new TestSink( @@ -40,58 +40,13 @@ public async void Invoke_LogsCorrectValuesWhenNotHandled() await middleware.Invoke(httpContext); // Assert - Assert.Single(sink.Scopes); - var scope = sink.Scopes[0]; - Assert.Equal(typeof(RouterMiddleware).FullName, scope.LoggerName); - Assert.Equal("RouterMiddleware.Invoke", scope.Scope.ToString()); - + Assert.Empty(sink.Scopes); Assert.Single(sink.Writes); - - var write = sink.Writes[0]; - Assert.Equal(typeof(RouterMiddleware).FullName, write.LoggerName); - Assert.Equal("RouterMiddleware.Invoke", write.Scope.ToString()); - var values = Assert.IsType(write.State); - Assert.Equal("RouterMiddleware.Invoke", values.Name); - Assert.Equal(false, values.Handled); - } - - [Fact] - public async void Invoke_DoesNotLogWhenDisabledAndNotHandled() - { - // Arrange - var isHandled = false; - - var sink = new TestSink( - TestSink.EnableWithTypeName, - TestSink.EnableWithTypeName); - var loggerFactory = new TestLoggerFactory(sink, enabled: false); - - var httpContext = new DefaultHttpContext(); - httpContext.ApplicationServices = new ServiceProvider(); - httpContext.RequestServices = httpContext.ApplicationServices; - - RequestDelegate next = (c) => - { - return Task.FromResult(null); - }; - - var router = new TestRouter(isHandled); - var middleware = new RouterMiddleware(next, loggerFactory, router); - - // Act - await middleware.Invoke(httpContext); - - // Assert - Assert.Single(sink.Scopes); - var scope = sink.Scopes[0]; - Assert.Equal(typeof(RouterMiddleware).FullName, scope.LoggerName); - Assert.Equal("RouterMiddleware.Invoke", scope.Scope.ToString()); - - Assert.Empty(sink.Writes); + Assert.Equal(expectedMessage, sink.Writes[0].State?.ToString()); } [Fact] - public async void Invoke_LogsCorrectValuesWhenHandled() + public async void Invoke_DoesNotLog_WhenHandled() { // Arrange var isHandled = true; @@ -111,63 +66,13 @@ public async void Invoke_LogsCorrectValuesWhenHandled() }; var router = new TestRouter(isHandled); - var middleware = new RouterMiddleware(next, loggerFactory, router); // Act await middleware.Invoke(httpContext); // Assert - // exists a BeginScope, verify contents - Assert.Single(sink.Scopes); - var scope = sink.Scopes[0]; - Assert.Equal(typeof(RouterMiddleware).FullName, scope.LoggerName); - Assert.Equal("RouterMiddleware.Invoke", scope.Scope.ToString()); - - Assert.Single(sink.Writes); - - var write = sink.Writes[0]; - Assert.Equal(typeof(RouterMiddleware).FullName, write.LoggerName); - Assert.Equal("RouterMiddleware.Invoke", write.Scope.ToString()); - Assert.Equal(typeof(RouterMiddlewareInvokeValues), write.State.GetType()); - var values = (RouterMiddlewareInvokeValues)write.State; - Assert.Equal("RouterMiddleware.Invoke", values.Name); - Assert.Equal(true, values.Handled); - } - - [Fact] - public async void Invoke_DoesNotLogWhenDisabledAndHandled() - { - // Arrange - var isHandled = true; - - var sink = new TestSink( - TestSink.EnableWithTypeName, - TestSink.EnableWithTypeName); - var loggerFactory = new TestLoggerFactory(sink, enabled: false); - - var httpContext = new DefaultHttpContext(); - httpContext.ApplicationServices = new ServiceProvider(); - httpContext.RequestServices = httpContext.ApplicationServices; - - RequestDelegate next = (c) => - { - return Task.FromResult(null); - }; - - var router = new TestRouter(isHandled); - var middleware = new RouterMiddleware(next, loggerFactory, router); - - // Act - await middleware.Invoke(httpContext); - - // Assert - // exists a BeginScope, verify contents - Assert.Single(sink.Scopes); - var scope = sink.Scopes[0]; - Assert.Equal(typeof(RouterMiddleware).FullName, scope.LoggerName); - Assert.Equal("RouterMiddleware.Invoke", scope.Scope.ToString()); - + Assert.Empty(sink.Scopes); Assert.Empty(sink.Writes); } diff --git a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTest.cs b/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTest.cs index 0c4c1ee8..a946ec7e 100644 --- a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTest.cs +++ b/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTest.cs @@ -11,9 +11,9 @@ using Microsoft.AspNet.Routing.Constraints; using Microsoft.AspNet.Routing.Logging; using Microsoft.AspNet.Testing; +using Microsoft.Framework.Logging.Internal; using Microsoft.Framework.Logging.Testing; using Microsoft.Framework.Logging; -using Microsoft.AspNet.WebUtilities; using Moq; using Xunit; @@ -23,20 +23,38 @@ public class TemplateRouteTest { private static IInlineConstraintResolver _inlineConstraintResolver = GetInlineConstraintResolver(); - private async Task> SetUp(bool enabled, string template, string requestPath) + private async Task> SetUp( + bool loggerEnabled, + string routeName, + string template, + string requestPath, + TestSink testSink = null) { - var sink = new TestSink( - TestSink.EnableWithTypeName, - TestSink.EnableWithTypeName); - var loggerFactory = new TestLoggerFactory(sink, enabled); + if (testSink == null) + { + testSink = new TestSink( + TestSink.EnableWithTypeName, + TestSink.EnableWithTypeName); + } + + var loggerFactory = new TestLoggerFactory(testSink, loggerEnabled); + + TemplateRoute route; + if (!string.IsNullOrEmpty(routeName)) + { + route = CreateRoute(routeName, template); + } + else + { + route = CreateRoute(template); + } - var route = CreateRoute(template); var context = CreateRouteContext(requestPath, loggerFactory); // Act await route.RouteAsync(context); - return Tuple.Create(sink, context); + return Tuple.Create(testSink, context); } [Fact] @@ -65,132 +83,81 @@ public void CreateTemplate_InlineConstraint_Regex_Malformed() public async Task RouteAsync_MatchSuccess_LogsCorrectValues() { // Arrange & Act + var routeName = "Default"; var template = "{controller}/{action}"; - var result = await SetUp(true, template, "/Home/Index"); - var sink = result.Item1; - var context = result.Item2; - - // Assert - Assert.Equal(1, sink.Scopes.Count); - var scope = sink.Scopes[0]; - Assert.Equal(typeof(TemplateRoute).FullName, scope.LoggerName); - Assert.Equal("TemplateRoute.RouteAsync", scope.Scope.ToString()); - - Assert.Equal(1, sink.Writes.Count); - - var write = sink.Writes[0]; - Assert.Equal(typeof(TemplateRoute).FullName, write.LoggerName); - Assert.Equal("TemplateRoute.RouteAsync", write.Scope.ToString()); - - // verify WriteCore state contents - var values = Assert.IsType(write.State); - Assert.Equal("TemplateRoute.RouteAsync", values.Name); - Assert.Equal("Home/Index", values.RequestPath); - Assert.Equal(template, values.Template); - Assert.NotNull(values.DefaultValues); - Assert.NotNull(values.ProducedValues); - Assert.Equal(true, values.MatchedTemplate); - Assert.Equal(true, values.MatchedConstraints); - Assert.Equal(true, values.Matched); - Assert.Equal(context.IsHandled, values.Handled); - } - - [Fact] - public async Task RouteAsync_MatchSuccess_DoesNotLogWhenDisabled() - { - // Arrange & Act - var template = "{controller}/{action}"; - var result = await SetUp(false, template, "/Home/Index"); + var result = await SetUp( + routeName: routeName, + template: template, + requestPath: "/Home/Index", + loggerEnabled: true); var sink = result.Item1; + var expected = "Request successfully matched the route with " + + $"name '{routeName}' and template '{template}'."; // Assert - Assert.Equal(1, sink.Scopes.Count); - var scope = sink.Scopes[0]; - Assert.Equal(typeof(TemplateRoute).FullName, scope.LoggerName); - Assert.Equal("TemplateRoute.RouteAsync", scope.Scope.ToString()); - - Assert.Equal(0, sink.Writes.Count); + Assert.Empty(sink.Scopes); + Assert.Single(sink.Writes); + Assert.Equal(expected, sink.Writes[0].State?.ToString()); } [Fact] public async Task RouteAsync_MatchFailOnValues_LogsCorrectValues() { // Arrange & Act + var routeName = "Default"; var template = "{controller}/{action}"; - var result = await SetUp(true, template, "/Home/Index/Failure"); - var sink = result.Item1; - var context = result.Item2; - - // Assert - Assert.Equal(1, sink.Scopes.Count); - var scope = sink.Scopes[0]; - Assert.Equal(typeof(TemplateRoute).FullName, scope.LoggerName); - Assert.Equal("TemplateRoute.RouteAsync", scope.Scope.ToString()); - - Assert.Equal(1, sink.Writes.Count); - - var write = sink.Writes[0]; - Assert.Equal(typeof(TemplateRoute).FullName, write.LoggerName); - Assert.Equal("TemplateRoute.RouteAsync", write.Scope.ToString()); - var values = Assert.IsType(write.State); - Assert.Equal("TemplateRoute.RouteAsync", values.Name); - Assert.Equal("Home/Index/Failure", values.RequestPath); - Assert.Equal(template, values.Template); - Assert.NotNull(values.DefaultValues); - Assert.Empty(values.ProducedValues); - Assert.Equal(false, values.MatchedTemplate); - Assert.Equal(false, values.MatchedConstraints); - Assert.Equal(false, values.Matched); - Assert.Equal(context.IsHandled, values.Handled); - } - - [Fact] - public async Task RouteAsync_MatchFailOnValues_DoesNotLogWhenDisabled() - { - // Arrange - var template = "{controller}/{action}"; - var result = await SetUp(false, template, "/Home/Index/Failure"); + var result = await SetUp( + routeName: routeName, + template: template, + requestPath: "/Home/Index/Failure", + loggerEnabled: true); var sink = result.Item1; + var expected = $"Request did not match the route with name '{routeName}' and template '{template}'."; // Assert - Assert.Equal(1, sink.Scopes.Count); - var scope = sink.Scopes[0]; - Assert.Equal(typeof(TemplateRoute).FullName, scope.LoggerName); - Assert.Equal("TemplateRoute.RouteAsync", scope.Scope.ToString()); - - Assert.Equal(0, sink.Writes.Count); + Assert.Empty(sink.Scopes); + Assert.Single(sink.Writes); + Assert.Equal(expected, sink.Writes[0].State?.ToString()); } [Fact] public async Task RouteAsync_MatchFailOnConstraints_LogsCorrectValues() { // Arrange & Act + var sink = new TestSink((writeEnabled) => true, (scopeEnabled) => true); var template = "{controller}/{action}/{id:int}"; - var result = await SetUp(true, template, "/Home/Index/Failure"); - var sink = result.Item1; - var context = result.Item2; - - // Assert - Assert.Equal(1, sink.Scopes.Count); - var scope = sink.Scopes[0]; - Assert.Equal(typeof(TemplateRoute).FullName, scope.LoggerName); - Assert.Equal("TemplateRoute.RouteAsync", scope.Scope.ToString()); - - Assert.Equal(1, sink.Writes.Count); - - var write = sink.Writes[0]; - Assert.Equal(typeof(TemplateRoute).FullName, write.LoggerName); - Assert.Equal("TemplateRoute.RouteAsync", write.Scope.ToString()); - var values = Assert.IsType(write.State); - Assert.Equal("TemplateRoute.RouteAsync", values.Name); - Assert.Equal("Home/Index/Failure", values.RequestPath); - Assert.Equal(template, values.Template); - Assert.NotNull(values.DefaultValues); - Assert.NotNull(values.ProducedValues); - Assert.Equal(true, values.MatchedTemplate); - Assert.Equal(false, values.MatchedConstraints); - Assert.Equal(false, values.Matched); - Assert.Equal(context.IsHandled, values.Handled); + var result = await SetUp( + routeName: "Default", + template: template, + requestPath: "/Home/Index/Failure", + loggerEnabled: true, + testSink: sink); + var expectedMessage = "Route value 'Failure' with key 'id' did not match the " + + $"constraint '{typeof(IntRouteConstraint).FullName}'."; + var expectedLogValues = new[] + { + new KeyValuePair("RouteValue", "Failure"), + new KeyValuePair("RouteKey", "id"), + new KeyValuePair("RouteConstraint", typeof(IntRouteConstraint).FullName) + }; + + // Assert + Assert.Empty(sink.Scopes); + Assert.Single(sink.Writes); + var sinkWrite = sink.Writes[0]; + var formattedLogValues = Assert.IsType(sinkWrite.State); + Assert.Equal(expectedMessage, formattedLogValues.ToString()); + var actualLogValues = formattedLogValues.GetValues(); + foreach(var expectedPair in expectedLogValues) + { + Assert.Contains( + actualLogValues, + (kvp) => + { + return string.Equals(expectedPair.Key, kvp.Key) + && string.Equals(expectedPair.Value?.ToString(), kvp.Value?.ToString()); + }); + } } [Fact] @@ -471,23 +438,6 @@ public async Task RouteAsync_CleansUpMergedRouteData_IfConstraintThrows() Assert.Same(originalDataTokens, context.RouteData.DataTokens); } - [Fact] - public async Task RouteAsync_MatchFailOnConstraints_DoesNotLogWhenDisabled() - { - // Arrange & Act - var template = "{controller}/{action}/{id:int}"; - var result = await SetUp(false, template, "/Home/Index/Failure"); - var sink = result.Item1; - - // Assert - Assert.Equal(1, sink.Scopes.Count); - var scope = sink.Scopes[0]; - Assert.Equal(typeof(TemplateRoute).FullName, scope.LoggerName); - Assert.Equal("TemplateRoute.RouteAsync", scope.Scope.ToString()); - - Assert.Equal(0, sink.Writes.Count); - } - [Fact] public async Task RouteAsync_InlineConstraint_OptionalParameter() { @@ -701,7 +651,7 @@ public async Task RouteAsync_InlineConstraint_OptionalParameter_ConstraintFails( Assert.False(context.IsHandled); } - #region Route Matching +#region Route Matching // PathString in HttpAbstractions guarantees a leading slash - so no value in testing other cases. [Fact] @@ -794,7 +744,7 @@ public async Task Match_Fails() public async Task Match_RejectedByHandler() { // Arrange - var route = CreateRoute("{controller}", accept: false); + var route = CreateRoute("{controller}", handleRequest: false); var context = CreateRouteContext("/Home"); // Act @@ -811,7 +761,7 @@ public async Task Match_RejectedByHandler() public async Task Match_RejectedByHandler_ClearsRouters() { // Arrange - var route = CreateRoute("{controller}", accept: false); + var route = CreateRoute("{controller}", handleRequest: false); var context = CreateRouteContext("/Home"); // Act @@ -826,7 +776,7 @@ public async Task Match_RejectedByHandler_ClearsRouters() public async Task Match_SetsRouters() { // Arrange - var target = CreateTarget(accept: true); + var target = CreateTarget(handleRequest: true); var route = CreateRoute(target, "{controller}"); var context = CreateRouteContext("/Home"); @@ -937,9 +887,9 @@ private static RouteContext CreateRouteContext(string requestPath, ILoggerFactor return new RouteContext(context.Object); } - #endregion +#endregion - #region Route Binding +#region Route Binding [Fact] public void GetVirtualPath_Success() @@ -1062,7 +1012,7 @@ public void GetVirtualPath_ReturnsDataTokens_WhenTargetReturnsNullVirtualPathDat public void GetVirtualPath_ValuesRejectedByHandler_StillGeneratesPath() { // Arrange - var route = CreateRoute("{controller}", accept: false); + var route = CreateRoute("{controller}", handleRequest: false); var context = CreateVirtualPathContext(new { controller = "Home" }); // Act @@ -1308,7 +1258,7 @@ public void GetVirtualPath_ConstraintsSeeAmbientValues() var route = CreateRoute( template: "slug/{controller}/{action}", defaults: null, - accept: true, + handleRequest: true, constraints: new { c = constraint }); var context = CreateVirtualPathContext( @@ -1339,7 +1289,7 @@ public void GetVirtualPath_ConstraintsDontSeeDefaults_WhenTheyArentParameters() var route = CreateRoute( template: "slug/{controller}/{action}", defaults: new { otherthing = "17" }, - accept: true, + handleRequest: true, constraints: new { c = constraint }); var context = CreateVirtualPathContext( @@ -1369,7 +1319,7 @@ public void GetVirtualPath_ConstraintsSeesDefault_WhenThereItsAParamter() var route = CreateRoute( template: "slug/{controller}/{action}", defaults: new { action = "Index" }, - accept: true, + handleRequest: true, constraints: new { c = constraint }); var context = CreateVirtualPathContext( @@ -1400,7 +1350,7 @@ public void GetVirtualPath_ConstraintsSeeDefaults_IfTheyAreSpecifiedOrAmbient() var route = CreateRoute( template: "slug/{controller}/{action}", defaults: new { otherthing = "17", thirdthing = "13" }, - accept: true, + handleRequest: true, constraints: new { c = constraint }); var context = CreateVirtualPathContext( @@ -1527,7 +1477,7 @@ public void GetVirtualPath_InlineConstraints_CompositeConstraint_FromConstructor var route = CreateRoute( template: "{controller}/{action}/{name:alpha}", defaults: null, - accept: true, + handleRequest: true, constraints: new { name = constraint }); var context = CreateVirtualPathContext( @@ -1549,7 +1499,7 @@ public void GetVirtualPath_OptionalParameter_ParameterPresentInValues() var route = CreateRoute( template: "{controller}/{action}/{name}.{format?}", defaults: null, - accept: true, + handleRequest: true, constraints: null); var context = CreateVirtualPathContext( @@ -1571,7 +1521,7 @@ public void GetVirtualPath_OptionalParameter_ParameterNotPresentInValues() var route = CreateRoute( template: "{controller}/{action}/{name}.{format?}", defaults: null, - accept: true, + handleRequest: true, constraints: null); var context = CreateVirtualPathContext( @@ -1593,7 +1543,7 @@ public void GetVirtualPath_OptionalParameter_ParameterPresentInValuesAndDefaults var route = CreateRoute( template: "{controller}/{action}/{name}.{format?}", defaults: new { format = "json" }, - accept: true, + handleRequest: true, constraints: null); var context = CreateVirtualPathContext( @@ -1615,7 +1565,7 @@ public void GetVirtualPath_OptionalParameter_ParameterNotPresentInValues_Present var route = CreateRoute( template: "{controller}/{action}/{name}.{format?}", defaults: new { format = "json" }, - accept: true, + handleRequest: true, constraints: null); var context = CreateVirtualPathContext( @@ -1637,7 +1587,7 @@ public void GetVirtualPath_OptionalParameter_ParameterNotPresentInTemplate_Prese var route = CreateRoute( template: "{controller}/{action}/{name}", defaults: null, - accept: true, + handleRequest: true, constraints: null); var context = CreateVirtualPathContext( @@ -1659,7 +1609,7 @@ public void GetVirtualPath_OptionalParameter_FollowedByDotAfterSlash_ParameterPr var route = CreateRoute( template: "{controller}/{action}/.{name?}", defaults: null, - accept: true, + handleRequest: true, constraints: null); var context = CreateVirtualPathContext( @@ -1681,7 +1631,7 @@ public void GetVirtualPath_OptionalParameter_FollowedByDotAfterSlash_ParameterNo var route = CreateRoute( template: "{controller}/{action}/.{name?}", defaults: null, - accept: true, + handleRequest: true, constraints: null); var context = CreateVirtualPathContext( @@ -1703,7 +1653,7 @@ public void GetVirtualPath_OptionalParameter_InSimpleSegment() var route = CreateRoute( template: "{controller}/{action}/{name?}", defaults: null, - accept: true, + handleRequest: true, constraints: null); var context = CreateVirtualPathContext( @@ -1744,9 +1694,9 @@ private static VirtualPathContext CreateVirtualPathContext(string routeName) return new VirtualPathContext(null, null, null, routeName); } - #endregion +#endregion - #region Route Registration +#region Route Registration public static IEnumerable DataTokens { @@ -1912,7 +1862,7 @@ public void RegisteringRouteWithRouteName_WithNullDefaultsAndConstraints_AddsThe Assert.Equal("RouteName", name); } - #endregion +#endregion // DataTokens test data for TemplateRoute.GetVirtualPath public static IEnumerable DataTokensTestData @@ -1939,18 +1889,30 @@ private static IRouteBuilder CreateRouteBuilder() return routeBuilder; } - private static TemplateRoute CreateRoute(string template, bool accept = true) + private static TemplateRoute CreateRoute(string routeName, string template, bool handleRequest = true) + { + return new TemplateRoute( + CreateTarget(handleRequest), + routeName, + template, + defaults: null, + constraints: null, + dataTokens: null, + inlineConstraintResolver: _inlineConstraintResolver); + } + + private static TemplateRoute CreateRoute(string template, bool handleRequest = true) { - return new TemplateRoute(CreateTarget(accept), template, _inlineConstraintResolver); + return new TemplateRoute(CreateTarget(handleRequest), template, _inlineConstraintResolver); } private static TemplateRoute CreateRoute(string template, object defaults, - bool accept = true, + bool handleRequest = true, object constraints = null, object dataTokens = null) { - return new TemplateRoute(CreateTarget(accept), + return new TemplateRoute(CreateTarget(handleRequest), template, new RouteValueDictionary(defaults), (constraints as IDictionary) ?? @@ -1984,17 +1946,17 @@ private static TemplateRoute CreateRoute( inlineConstraintResolver: _inlineConstraintResolver); } - private static IRouter CreateTarget(bool accept = true) + private static IRouter CreateTarget(bool handleRequest = true) { var target = new Mock(MockBehavior.Strict); target .Setup(e => e.GetVirtualPath(It.IsAny())) - .Callback(c => c.IsBound = accept) + .Callback(c => c.IsBound = handleRequest) .Returns(rc => null); target .Setup(e => e.RouteAsync(It.IsAny())) - .Callback((c) => c.IsHandled = accept) + .Callback((c) => c.IsHandled = handleRequest) .Returns(Task.FromResult(null)); return target.Object;