From 2a8ac595d8630d77706e08bedc04d3fc70d88281 Mon Sep 17 00:00:00 2001 From: SonjaKhan Date: Mon, 6 Oct 2014 13:56:23 -0700 Subject: [PATCH] Refactoring ILogger, see aspnet/Logging#3 --- Routing.sln | 2 +- .../Logging/LoggerExtensions.cs | 4 +- .../ConstraintMatcherTest.cs | 147 ++++++++++-------- .../Logging/NullLogger.cs | 6 +- .../Logging/TestLogger.cs | 17 +- .../Logging/TestLoggerFactory.cs | 6 +- .../Logging/TestSink.cs | 12 +- .../{WriteCoreContext.cs => WriteContext.cs} | 2 +- .../RouteCollectionTest.cs | 107 +++++++------ .../RouterMiddlewareTest.cs | 99 +++++++++--- .../Template/TemplateRouteTest.cs | 135 +++++++++------- 11 files changed, 330 insertions(+), 207 deletions(-) rename test/Microsoft.AspNet.Routing.Tests/Logging/{WriteCoreContext.cs => WriteContext.cs} (94%) diff --git a/Routing.sln b/Routing.sln index aef9202a9b..4cfe492c64 100644 --- a/Routing.sln +++ b/Routing.sln @@ -1,7 +1,7 @@  Microsoft Visual Studio Solution File, Format Version 12.00 # Visual Studio 14 -VisualStudioVersion = 14.0.22013.1 +VisualStudioVersion = 14.0.22115.0 MinimumVisualStudioVersion = 10.0.40219.1 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{0E966C37-7334-4D96-AAF6-9F49FBD166E3}" EndProject diff --git a/src/Microsoft.AspNet.Routing/Logging/LoggerExtensions.cs b/src/Microsoft.AspNet.Routing/Logging/LoggerExtensions.cs index ebcdca6744..75e8582ff3 100644 --- a/src/Microsoft.AspNet.Routing/Logging/LoggerExtensions.cs +++ b/src/Microsoft.AspNet.Routing/Logging/LoggerExtensions.cs @@ -7,9 +7,9 @@ namespace Microsoft.AspNet.Routing.Logging { internal static class LoggerExtensions { - public static bool WriteValues([NotNull] this ILogger logger, object values) + public static void WriteValues([NotNull] this ILogger logger, object values) { - return logger.WriteCore( + logger.Write( eventType: TraceType.Information, eventId: 0, state: values, diff --git a/test/Microsoft.AspNet.Routing.Tests/ConstraintMatcherTest.cs b/test/Microsoft.AspNet.Routing.Tests/ConstraintMatcherTest.cs index 34a168c8fa..3e3f958a7b 100644 --- a/test/Microsoft.AspNet.Routing.Tests/ConstraintMatcherTest.cs +++ b/test/Microsoft.AspNet.Routing.Tests/ConstraintMatcherTest.cs @@ -14,14 +14,14 @@ namespace Microsoft.AspNet.Routing public class ConstraintMatcherTest { #if ASPNET50 + private const string _name = "name"; + [Fact] public void MatchUrlGeneration_DoesNotLogData() { // Arrange - var name = "name"; - var sink = new TestSink(); - var logger = new TestLogger(name, sink); + var logger = new TestLogger(_name, sink, enabled: true); var routeValueDictionary = new RouteValueDictionary(new { a = "value", b = "value" }); var constraints = new Dictionary @@ -41,62 +41,40 @@ namespace Microsoft.AspNet.Routing // Assert // There are no BeginScopes called. - Assert.Equal(0, sink.Scopes.Count); + Assert.Empty(sink.Scopes); // There are no WriteCores called. - Assert.Equal(0, sink.Writes.Count); + Assert.Empty(sink.Writes); } [Fact] public void MatchFail_LogsCorrectData() { - // Arrange - var name = "name"; - - var sink = new TestSink(); - var logger = new TestLogger(name, sink); - - var routeValueDictionary = new RouteValueDictionary(new { a = "value", b = "value" }); + // Arrange & Act var constraints = new Dictionary { {"a", new PassConstraint()}, {"b", new FailConstraint()} }; - - // Act - RouteConstraintMatcher.Match( - constraints: constraints, - routeValues: routeValueDictionary, - httpContext: new Mock().Object, - route: new Mock().Object, - routeDirection: RouteDirection.IncomingRequest, - logger: logger); + var sink = SetUpMatch(constraints, true); // Assert // There are no begin scopes called. - Assert.Equal(0, sink.Scopes.Count); + Assert.Empty(sink.Scopes); - // There are two records for IsEnabled and two for WriteCore. - Assert.Equal(4, sink.Writes.Count); + // There are two records for WriteCore. + Assert.Equal(2, sink.Writes.Count); - var enabled = sink.Writes[0]; - Assert.Equal(name, enabled.LoggerName); - Assert.Null(enabled.State); - - var write = sink.Writes[1]; - Assert.Equal(name, write.LoggerName); + 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); - enabled = sink.Writes[2]; - Assert.Equal(name, enabled.LoggerName); - Assert.Null(enabled.State); - - write = sink.Writes[3]; - Assert.Equal(name, write.LoggerName); + 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); @@ -104,56 +82,53 @@ namespace Microsoft.AspNet.Routing 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); + } + [Fact] public void MatchSuccess_LogsCorrectData() { - // Arrange - var name = "name"; - - var sink = new TestSink(); - var logger = new TestLogger(name, sink); - - var routeValueDictionary = new RouteValueDictionary(new { a = "value", b = "value" }); + // Arrange & Act var constraints = new Dictionary { {"a", new PassConstraint()}, {"b", new PassConstraint()} }; - - // Act - RouteConstraintMatcher.Match( - constraints: constraints, - routeValues: routeValueDictionary, - httpContext: new Mock().Object, - route: new Mock().Object, - routeDirection: RouteDirection.IncomingRequest, - logger: logger); + var sink = SetUpMatch(constraints, true); // Assert // There are no begin scopes called. - Assert.Equal(0, sink.Scopes.Count); + Assert.Empty(sink.Scopes); - // There are two records for IsEnabled and two for WriteCore. - Assert.Equal(4, sink.Writes.Count); + // There are two records WriteCore. + Assert.Equal(2, sink.Writes.Count); - var enabled = sink.Writes[0]; - Assert.Equal(name, enabled.LoggerName); - Assert.Null(enabled.State); - - var write = sink.Writes[1]; - Assert.Equal(name, write.LoggerName); + 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); - enabled = sink.Writes[2]; - Assert.Equal(name, enabled.LoggerName); - Assert.Null(enabled.State); - - write = sink.Writes[3]; - Assert.Equal(name, write.LoggerName); + 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); @@ -161,6 +136,25 @@ namespace Microsoft.AspNet.Routing Assert.Equal(true, values.Matched); } + [Fact] + public void MatchSuccess_DisabledLoggerDoesNotLog() + { + // Arrange & Act + var constraints = new Dictionary + { + {"a", new PassConstraint()}, + {"b", new PassConstraint()} + }; + 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); + } + [Fact] public void ReturnsTrueOnValidConstraints() { @@ -272,6 +266,25 @@ namespace Microsoft.AspNet.Routing routeDirection: RouteDirection.IncomingRequest, logger: NullLogger.Instance)); } + + private TestSink SetUpMatch(Dictionary constraints, bool enabled) + { + // Arrange + var sink = new TestSink(); + var logger = new TestLogger(_name, sink, enabled); + + var routeValueDictionary = new RouteValueDictionary(new { a = "value", b = "value" }); + + // Act + RouteConstraintMatcher.Match( + constraints: constraints, + routeValues: routeValueDictionary, + httpContext: new Mock().Object, + route: new Mock().Object, + routeDirection: RouteDirection.IncomingRequest, + logger: logger); + return sink; + } #endif private class PassConstraint : IRouteConstraint diff --git a/test/Microsoft.AspNet.Routing.Tests/Logging/NullLogger.cs b/test/Microsoft.AspNet.Routing.Tests/Logging/NullLogger.cs index f92dc40c34..1049ec0c45 100644 --- a/test/Microsoft.AspNet.Routing.Tests/Logging/NullLogger.cs +++ b/test/Microsoft.AspNet.Routing.Tests/Logging/NullLogger.cs @@ -15,7 +15,11 @@ namespace Microsoft.AspNet.Routing return NullDisposable.Instance; } - public bool WriteCore(TraceType eventType, int eventId, object state, Exception exception, Func formatter) + public void Write(TraceType eventType, int eventId, object state, Exception exception, Func formatter) + { + } + + public bool IsEnabled(TraceType eventType) { return false; } diff --git a/test/Microsoft.AspNet.Routing.Tests/Logging/TestLogger.cs b/test/Microsoft.AspNet.Routing.Tests/Logging/TestLogger.cs index 2627d8463b..c42a3405c5 100644 --- a/test/Microsoft.AspNet.Routing.Tests/Logging/TestLogger.cs +++ b/test/Microsoft.AspNet.Routing.Tests/Logging/TestLogger.cs @@ -11,12 +11,14 @@ namespace Microsoft.AspNet.Routing private object _scope; private TestSink _sink; private string _name; + private bool _enabled; - public TestLogger(string name, TestSink sink) - { + public TestLogger(string name, TestSink sink, bool enabled) + { _sink = sink; _name = name; - } + _enabled = enabled; + } public string Name { get; set; } @@ -33,9 +35,9 @@ namespace Microsoft.AspNet.Routing return NullDisposable.Instance; } - public bool WriteCore(TraceType eventType, int eventId, object state, Exception exception, Func formatter) + public void Write(TraceType eventType, int eventId, object state, Exception exception, Func formatter) { - _sink.Write(new WriteCoreContext() + _sink.Write(new WriteContext() { EventType = eventType, EventId = eventId, @@ -45,8 +47,11 @@ namespace Microsoft.AspNet.Routing LoggerName = _name, Scope = _scope }); + } - return true; + public bool IsEnabled(TraceType eventType) + { + return _enabled; } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Routing.Tests/Logging/TestLoggerFactory.cs b/test/Microsoft.AspNet.Routing.Tests/Logging/TestLoggerFactory.cs index 49fe0db326..18e4332a22 100644 --- a/test/Microsoft.AspNet.Routing.Tests/Logging/TestLoggerFactory.cs +++ b/test/Microsoft.AspNet.Routing.Tests/Logging/TestLoggerFactory.cs @@ -8,15 +8,17 @@ namespace Microsoft.AspNet.Routing public class TestLoggerFactory : ILoggerFactory { private TestSink _sink; + private bool _enabled; - public TestLoggerFactory(TestSink sink) + public TestLoggerFactory(TestSink sink, bool enabled) { _sink = sink; + _enabled = enabled; } public ILogger Create(string name) { - return new TestLogger(name, _sink); + return new TestLogger(name, _sink, _enabled); } public void AddProvider(ILoggerProvider provider) diff --git a/test/Microsoft.AspNet.Routing.Tests/Logging/TestSink.cs b/test/Microsoft.AspNet.Routing.Tests/Logging/TestSink.cs index 4c50f01caa..824166300f 100644 --- a/test/Microsoft.AspNet.Routing.Tests/Logging/TestSink.cs +++ b/test/Microsoft.AspNet.Routing.Tests/Logging/TestSink.cs @@ -9,25 +9,25 @@ namespace Microsoft.AspNet.Routing public class TestSink { public TestSink( - Func writeEnabled = null, + Func writeEnabled = null, Func beginEnabled = null) { WriteEnabled = writeEnabled; BeginEnabled = beginEnabled; Scopes = new List(); - Writes = new List(); + Writes = new List(); } - public Func WriteEnabled { get; set; } + public Func WriteEnabled { get; set; } public Func BeginEnabled { get; set; } public List Scopes { get; set; } - public List Writes { get; set; } + public List Writes { get; set; } - public void Write(WriteCoreContext context) + public void Write(WriteContext context) { if (WriteEnabled == null || WriteEnabled(context)) { @@ -43,7 +43,7 @@ namespace Microsoft.AspNet.Routing } } - public static bool EnableWithTypeName(WriteCoreContext context) + public static bool EnableWithTypeName(WriteContext context) { return context.LoggerName.Equals(typeof(T).FullName); } diff --git a/test/Microsoft.AspNet.Routing.Tests/Logging/WriteCoreContext.cs b/test/Microsoft.AspNet.Routing.Tests/Logging/WriteContext.cs similarity index 94% rename from test/Microsoft.AspNet.Routing.Tests/Logging/WriteCoreContext.cs rename to test/Microsoft.AspNet.Routing.Tests/Logging/WriteContext.cs index ddab5afb47..4466b344d3 100644 --- a/test/Microsoft.AspNet.Routing.Tests/Logging/WriteCoreContext.cs +++ b/test/Microsoft.AspNet.Routing.Tests/Logging/WriteContext.cs @@ -6,7 +6,7 @@ using Microsoft.Framework.Logging; namespace Microsoft.AspNet.Routing { - public class WriteCoreContext + public class WriteContext { public TraceType EventType { get; set; } diff --git a/test/Microsoft.AspNet.Routing.Tests/RouteCollectionTest.cs b/test/Microsoft.AspNet.Routing.Tests/RouteCollectionTest.cs index 3aa670db62..b416ca4384 100644 --- a/test/Microsoft.AspNet.Routing.Tests/RouteCollectionTest.cs +++ b/test/Microsoft.AspNet.Routing.Tests/RouteCollectionTest.cs @@ -19,36 +19,18 @@ namespace Microsoft.AspNet.Routing [Fact] public async Task RouteAsync_LogsCorrectValuesWhenHandled() { - // Arrange - var sink = new TestSink( - TestSink.EnableWithTypeName, - TestSink.EnableWithTypeName); - var loggerFactory = new TestLoggerFactory(sink); - - var routes = new RouteCollection(); - var route = CreateRoute(accept: true); - routes.Add(route.Object); - - var context = CreateRouteContext("/Cool", loggerFactory); - - // Act - await routes.RouteAsync(context); + // Arrange & Act + var sink = await SetUp(enabled: true, handled: true); // Assert - Assert.Equal(1, sink.Scopes.Count); + Assert.Single(sink.Scopes); var scope = sink.Scopes[0]; Assert.Equal(typeof(RouteCollection).FullName, scope.LoggerName); Assert.Equal("RouteCollection.RouteAsync", scope.Scope); - // There is a record for IsEnabled and one for WriteCore. - Assert.Equal(2, sink.Writes.Count); + Assert.Single(sink.Writes); - var enabled = sink.Writes[0]; - Assert.Equal(typeof(RouteCollection).FullName, enabled.LoggerName); - Assert.Equal("RouteCollection.RouteAsync", enabled.Scope); - Assert.Null(enabled.State); - - var write = sink.Writes[1]; + var write = sink.Writes[0]; Assert.Equal(typeof(RouteCollection).FullName, write.LoggerName); Assert.Equal("RouteCollection.RouteAsync", write.Scope); var values = Assert.IsType(write.State); @@ -58,38 +40,36 @@ namespace Microsoft.AspNet.Routing } [Fact] - public async Task RouteAsync_LogsCorrectValuesWhenNotHandled() + public async Task RouteAsync_DoesNotLogWhenDisabledAndHandled() { - // Arrange - var sink = new TestSink( - TestSink.EnableWithTypeName, - TestSink.EnableWithTypeName); - var loggerFactory = new TestLoggerFactory(sink); - - var routes = new RouteCollection(); - var route = CreateRoute(accept: false); - routes.Add(route.Object); - - var context = CreateRouteContext("/Cool", loggerFactory); - - // Act - await routes.RouteAsync(context); + // Arrange & Act + var sink = await SetUp(enabled: false, handled: true); // Assert - Assert.Equal(1, sink.Scopes.Count); + Assert.Single(sink.Scopes); + var scope = sink.Scopes[0]; + Assert.Equal(typeof(RouteCollection).FullName, scope.LoggerName); + Assert.Equal("RouteCollection.RouteAsync", scope.Scope); + + 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); // There is a record for IsEnabled and one for WriteCore. - Assert.Equal(2, sink.Writes.Count); + Assert.Single(sink.Writes); - var enabled = sink.Writes[0]; - Assert.Equal(typeof(RouteCollection).FullName, enabled.LoggerName); - Assert.Equal("RouteCollection.RouteAsync", enabled.Scope); - Assert.Null(enabled.State); - - var write = sink.Writes[1]; + var write = sink.Writes[0]; Assert.Equal(typeof(RouteCollection).FullName, write.LoggerName); Assert.Equal("RouteCollection.RouteAsync", write.Scope); var values = Assert.IsType(write.State); @@ -98,6 +78,21 @@ namespace Microsoft.AspNet.Routing 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); + + Assert.Empty(sink.Writes); + } + [Fact] public async Task RouteAsync_FirstMatches() { @@ -228,6 +223,26 @@ namespace Microsoft.AspNet.Routing Assert.Equal("The supplied route name 'ambiguousRoute' is ambiguous and matched more than one route.", ex.Message); } + private static async Task SetUp(bool enabled, bool handled) + { + // Arrange + var sink = new TestSink( + TestSink.EnableWithTypeName, + TestSink.EnableWithTypeName); + var loggerFactory = new TestLoggerFactory(sink, enabled); + + var routes = new RouteCollection(); + var route = CreateRoute(accept: handled); + routes.Add(route.Object); + + var context = CreateRouteContext("/Cool", loggerFactory); + + // Act + await routes.RouteAsync(context); + + return sink; + } + private static RouteCollection GetRouteCollectionWithNamedRoutes(IEnumerable routeNames) { var routes = new RouteCollection(); diff --git a/test/Microsoft.AspNet.Routing.Tests/RouterMiddlewareTest.cs b/test/Microsoft.AspNet.Routing.Tests/RouterMiddlewareTest.cs index f3a4b27198..d36984efb0 100644 --- a/test/Microsoft.AspNet.Routing.Tests/RouterMiddlewareTest.cs +++ b/test/Microsoft.AspNet.Routing.Tests/RouterMiddlewareTest.cs @@ -25,7 +25,7 @@ namespace Microsoft.AspNet.Routing var sink = new TestSink( TestSink.EnableWithTypeName, TestSink.EnableWithTypeName); - var loggerFactory = new TestLoggerFactory(sink); + var loggerFactory = new TestLoggerFactory(sink, enabled: true); var mockContext = new Mock(MockBehavior.Strict); mockContext.Setup(m => m.RequestServices.GetService(typeof(ILoggerFactory))) @@ -43,20 +43,14 @@ namespace Microsoft.AspNet.Routing await middleware.Invoke(mockContext.Object); // Assert - Assert.Equal(1, sink.Scopes.Count); + Assert.Single(sink.Scopes); var scope = sink.Scopes[0]; Assert.Equal(typeof(RouterMiddleware).FullName, scope.LoggerName); Assert.Equal("RouterMiddleware.Invoke", scope.Scope); - // There is a record for IsEnabled and one for WriteCore. - Assert.Equal(2, sink.Writes.Count); + Assert.Single(sink.Writes); - var enabled = sink.Writes[0]; - Assert.Equal(typeof(RouterMiddleware).FullName, enabled.LoggerName); - Assert.Equal("RouterMiddleware.Invoke", enabled.Scope); - Assert.Null(enabled.State); - - var write = sink.Writes[1]; + var write = sink.Writes[0]; Assert.Equal(typeof(RouterMiddleware).FullName, write.LoggerName); Assert.Equal("RouterMiddleware.Invoke", write.Scope); var values = Assert.IsType(write.State); @@ -64,6 +58,41 @@ namespace Microsoft.AspNet.Routing 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 mockContext = new Mock(MockBehavior.Strict); + mockContext.Setup(m => m.RequestServices.GetService(typeof(ILoggerFactory))) + .Returns(loggerFactory); + + RequestDelegate next = (c) => + { + return Task.FromResult(null); + }; + + var router = new TestRouter(isHandled); + var middleware = new RouterMiddleware(next, router); + + // Act + await middleware.Invoke(mockContext.Object); + + // Assert + Assert.Single(sink.Scopes); + var scope = sink.Scopes[0]; + Assert.Equal(typeof(RouterMiddleware).FullName, scope.LoggerName); + Assert.Equal("RouterMiddleware.Invoke", scope.Scope); + + Assert.Empty(sink.Writes); + } + [Fact] public async void Invoke_LogsCorrectValuesWhenHandled() { @@ -73,7 +102,7 @@ namespace Microsoft.AspNet.Routing var sink = new TestSink( TestSink.EnableWithTypeName, TestSink.EnableWithTypeName); - var loggerFactory = new TestLoggerFactory(sink); + var loggerFactory = new TestLoggerFactory(sink, enabled: true); var mockContext = new Mock(MockBehavior.Strict); mockContext.Setup(m => m.RequestServices.GetService(typeof(ILoggerFactory))) @@ -92,20 +121,14 @@ namespace Microsoft.AspNet.Routing // Assert // exists a BeginScope, verify contents - Assert.Equal(1, sink.Scopes.Count); + Assert.Single(sink.Scopes); var scope = sink.Scopes[0]; Assert.Equal(typeof(RouterMiddleware).FullName, scope.LoggerName); Assert.Equal("RouterMiddleware.Invoke", scope.Scope); - // There is a record for IsEnabled and one for WriteCore. - Assert.Equal(2, sink.Writes.Count); + Assert.Single(sink.Writes); - var enabled = sink.Writes[0]; - Assert.Equal(typeof(RouterMiddleware).FullName, enabled.LoggerName); - Assert.Equal("RouterMiddleware.Invoke", enabled.Scope); - Assert.Null(enabled.State); - - var write = sink.Writes[1]; + var write = sink.Writes[0]; Assert.Equal(typeof(RouterMiddleware).FullName, write.LoggerName); Assert.Equal("RouterMiddleware.Invoke", write.Scope); Assert.Equal(typeof(RouterMiddlewareInvokeValues), write.State.GetType()); @@ -113,6 +136,42 @@ namespace Microsoft.AspNet.Routing 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 mockContext = new Mock(MockBehavior.Strict); + mockContext.Setup(m => m.RequestServices.GetService(typeof(ILoggerFactory))) + .Returns(loggerFactory); + + RequestDelegate next = (c) => + { + return Task.FromResult(null); + }; + + var router = new TestRouter(isHandled); + var middleware = new RouterMiddleware(next, router); + + // Act + await middleware.Invoke(mockContext.Object); + + // 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); + + Assert.Empty(sink.Writes); + } #endif private class TestRouter : IRouter diff --git a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTest.cs b/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTest.cs index 909249e5ef..5de78131d0 100644 --- a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTest.cs +++ b/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTest.cs @@ -20,38 +20,40 @@ namespace Microsoft.AspNet.Routing.Template { private static IInlineConstraintResolver _inlineConstraintResolver = GetInlineConstraintResolver(); - [Fact] - public async Task RouteAsync_MatchSuccess_LogsCorrectValues() + private async Task> SetUp(bool enabled, string template, string requestPath) { - // Arrange var sink = new TestSink( TestSink.EnableWithTypeName, TestSink.EnableWithTypeName); - var loggerFactory = new TestLoggerFactory(sink); - - var template = "{controller}/{action}"; + var loggerFactory = new TestLoggerFactory(sink, enabled); var route = CreateRoute(template); - var context = CreateRouteContext("/Home/Index", loggerFactory); + var context = CreateRouteContext(requestPath, loggerFactory); // Act await route.RouteAsync(context); + return Tuple.Create(sink, context); + } + + [Fact] + public async Task RouteAsync_MatchSuccess_LogsCorrectValues() + { + // Arrange & Act + 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); - // There is a record for IsEnabled and one for WriteCore. - Assert.Equal(2, sink.Writes.Count); + Assert.Equal(1, sink.Writes.Count); - var enabled = sink.Writes[0]; - Assert.Equal(typeof(TemplateRoute).FullName, enabled.LoggerName); - Assert.Equal("TemplateRoute.RouteAsync", enabled.Scope); - Assert.Null(enabled.State); - - var write = sink.Writes[1]; + var write = sink.Writes[0]; Assert.Equal(typeof(TemplateRoute).FullName, write.LoggerName); Assert.Equal("TemplateRoute.RouteAsync", write.Scope); @@ -69,21 +71,12 @@ namespace Microsoft.AspNet.Routing.Template } [Fact] - public async Task RouteAsync_MatchFailOnValues_LogsCorrectValues() + public async Task RouteAsync_MatchSuccess_DoesNotLogWhenDisabled() { - // Arrange - var sink = new TestSink( - TestSink.EnableWithTypeName, - TestSink.EnableWithTypeName); - var loggerFactory = new TestLoggerFactory(sink); - + // Arrange & Act var template = "{controller}/{action}"; - - var route = CreateRoute(template); - var context = CreateRouteContext("/Home/Index/Failure", loggerFactory); - - // Act - await route.RouteAsync(context); + var result = await SetUp(false, template, "/Home/Index"); + var sink = result.Item1; // Assert Assert.Equal(1, sink.Scopes.Count); @@ -91,15 +84,27 @@ namespace Microsoft.AspNet.Routing.Template Assert.Equal(typeof(TemplateRoute).FullName, scope.LoggerName); Assert.Equal("TemplateRoute.RouteAsync", scope.Scope); - // There is a record for IsEnabled and one for WriteCore. - Assert.Equal(2, sink.Writes.Count); + Assert.Equal(0, sink.Writes.Count); + } - var enabled = sink.Writes[0]; - Assert.Equal(typeof(TemplateRoute).FullName, enabled.LoggerName); - Assert.Equal("TemplateRoute.RouteAsync", enabled.Scope); - Assert.Null(enabled.State); + [Fact] + public async Task RouteAsync_MatchFailOnValues_LogsCorrectValues() + { + // Arrange & Act + var template = "{controller}/{action}"; + var result = await SetUp(true, template, "/Home/Index/Failure"); + var sink = result.Item1; + var context = result.Item2; - var write = sink.Writes[1]; + // 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); + + Assert.Equal(1, sink.Writes.Count); + + var write = sink.Writes[0]; Assert.Equal(typeof(TemplateRoute).FullName, write.LoggerName); Assert.Equal("TemplateRoute.RouteAsync", write.Scope); var values = Assert.IsType(write.State); @@ -115,21 +120,12 @@ namespace Microsoft.AspNet.Routing.Template } [Fact] - public async Task RouteAsync_MatchFailOnConstraints_LogsCorrectValues() + public async Task RouteAsync_MatchFailOnValues_DoesNotLogWhenDisabled() { // Arrange - var sink = new TestSink( - TestSink.EnableWithTypeName, - TestSink.EnableWithTypeName); - var loggerFactory = new TestLoggerFactory(sink); - - var template = "{controller}/{action}/{id:int}"; - - var route = CreateRoute(template); - var context = CreateRouteContext("/Home/Index/Failure", loggerFactory); - - // Act - await route.RouteAsync(context); + var template = "{controller}/{action}"; + var result = await SetUp(false, template, "/Home/Index/Failure"); + var sink = result.Item1; // Assert Assert.Equal(1, sink.Scopes.Count); @@ -137,15 +133,27 @@ namespace Microsoft.AspNet.Routing.Template Assert.Equal(typeof(TemplateRoute).FullName, scope.LoggerName); Assert.Equal("TemplateRoute.RouteAsync", scope.Scope); - // There is a record for IsEnabled and one for WriteCore. - Assert.Equal(2, sink.Writes.Count); + Assert.Equal(0, sink.Writes.Count); + } - var enabled = sink.Writes[0]; - Assert.Equal(typeof(TemplateRoute).FullName, enabled.LoggerName); - Assert.Equal("TemplateRoute.RouteAsync", enabled.Scope); - Assert.Null(enabled.State); + [Fact] + public async Task RouteAsync_MatchFailOnConstraints_LogsCorrectValues() + { + // Arrange & Act + var template = "{controller}/{action}/{id:int}"; + var result = await SetUp(true, template, "/Home/Index/Failure"); + var sink = result.Item1; + var context = result.Item2; - var write = sink.Writes[1]; + // 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); + + Assert.Equal(1, sink.Writes.Count); + + var write = sink.Writes[0]; Assert.Equal(typeof(TemplateRoute).FullName, write.LoggerName); Assert.Equal("TemplateRoute.RouteAsync", write.Scope); var values = Assert.IsType(write.State); @@ -160,6 +168,23 @@ namespace Microsoft.AspNet.Routing.Template Assert.Equal(context.IsHandled, values.Handled); } + [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); + + Assert.Equal(0, sink.Writes.Count); + } + #region Route Matching // PathString in HttpAbstractions guarantees a leading slash - so no value in testing other cases.