Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Commit

Permalink
[Fixes #4291] Simplified MvcRouteHandler code
Browse files Browse the repository at this point in the history
  • Loading branch information
@ajaybhargavb
ajaybhargavb committed May 27, 2016
1 parent 9963359 commit 4cef233
Show file tree
Hide file tree
Showing 5 changed files with 212 additions and 153 deletions.
90 changes: 57 additions & 33 deletions src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvoker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,45 +112,69 @@ public ControllerActionInvoker(

public virtual async Task InvokeAsync()
{
await InvokeAllAuthorizationFiltersAsync();

// If Authorization Filters return a result, it's a short circuit because
// authorization failed. We don't execute Result Filters around the result.
Debug.Assert(_authorizationContext != null);
if (_authorizationContext.Result != null)
{
await InvokeResultAsync(_authorizationContext.Result);
return;
}

try
{
await InvokeAllResourceFiltersAsync();
}
finally
{
// Release the instance after all filters have run. We don't need to surround
// Authorizations filters because the instance will be created much later than
// that.
if (_controller != null)
_diagnosticSource.BeforeAction(
_controllerContext.ActionDescriptor,
_controllerContext.HttpContext,
_controllerContext.RouteData);

using (_logger.ActionScope(_controllerContext.ActionDescriptor))
{
_controllerFactory.ReleaseController(_controllerContext, _controller);
_logger.ExecutingAction(_controllerContext.ActionDescriptor);

var startTimestamp = _logger.IsEnabled(LogLevel.Information) ? Stopwatch.GetTimestamp() : 0;

await InvokeAllAuthorizationFiltersAsync();

// If Authorization Filters return a result, it's a short circuit because
// authorization failed. We don't execute Result Filters around the result.
Debug.Assert(_authorizationContext != null);
if (_authorizationContext.Result != null)
{
await InvokeResultAsync(_authorizationContext.Result);
return;
}

try
{
await InvokeAllResourceFiltersAsync();
}
finally
{
// Release the instance after all filters have run. We don't need to surround
// Authorizations filters because the instance will be created much later than
// that.
if (_controller != null)
{
_controllerFactory.ReleaseController(_controllerContext, _controller);
}
}

// We've reached the end of resource filters. If there's an unhandled exception on the context then
// it should be thrown and middleware has a chance to handle it.
Debug.Assert(_resourceExecutedContext != null);
if (_resourceExecutedContext.Exception != null && !_resourceExecutedContext.ExceptionHandled)
{
if (_resourceExecutedContext.ExceptionDispatchInfo == null)
{
throw _resourceExecutedContext.Exception;
}
else
{
_resourceExecutedContext.ExceptionDispatchInfo.Throw();
}
}

_logger.ExecutedAction(_controllerContext.ActionDescriptor, startTimestamp);
}
}

// We've reached the end of resource filters. If there's an unhandled exception on the context then
// it should be thrown and middleware has a chance to handle it.
Debug.Assert(_resourceExecutedContext != null);
if (_resourceExecutedContext.Exception != null && !_resourceExecutedContext.ExceptionHandled)
finally
{
if (_resourceExecutedContext.ExceptionDispatchInfo == null)
{
throw _resourceExecutedContext.Exception;
}
else
{
_resourceExecutedContext.ExceptionDispatchInfo.Throw();
}
_diagnosticSource.AfterAction(
_controllerContext.ActionDescriptor,
_controllerContext.HttpContext,
_controllerContext.RouteData);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreLoggerExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,12 @@ static MvcCoreLoggerExtensions()
_actionMethodExecuting = LoggerMessage.Define<string, string[], ModelValidationState>(
LogLevel.Information,
1,
"Executing action method {ActionName} with arguments ({Arguments}) - ModelState is {ValidationState}'");
"Executing action method {ActionName} with arguments ({Arguments}) - ModelState is {ValidationState}");

_actionMethodExecuted = LoggerMessage.Define<string, string>(
LogLevel.Debug,
2,
"Executed action method {ActionName}, returned result {ActionResult}.'");
"Executed action method {ActionName}, returned result {ActionResult}.");

_ambiguousActions = LoggerMessage.Define<string>(
LogLevel.Error,
Expand Down
51 changes: 13 additions & 38 deletions src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcRouteHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
using System;
using System.Diagnostics;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.Abstractions;
using Microsoft.AspNetCore.Mvc.Core;
using Microsoft.AspNetCore.Mvc.Infrastructure;
using Microsoft.AspNetCore.Routing;
Expand Down Expand Up @@ -87,46 +85,23 @@ public Task RouteAsync(RouteContext context)
context.RouteData.Values.Remove(TreeRouter.RouteGroupKey);
}

context.Handler = (c) => InvokeActionAsync(c, actionDescriptor);
return TaskCache.CompletedTask;
}

private async Task InvokeActionAsync(HttpContext httpContext, ActionDescriptor actionDescriptor)
{
var routeData = httpContext.GetRouteData();
try
var routeData = context.HttpContext.GetRouteData();
var actionContext = new ActionContext(context.HttpContext, routeData, actionDescriptor);
if (_actionContextAccessor != null)
{
_diagnosticSource.BeforeAction(actionDescriptor, httpContext, routeData);

using (_logger.ActionScope(actionDescriptor))
{
_logger.ExecutingAction(actionDescriptor);

var startTimestamp = _logger.IsEnabled(LogLevel.Information) ? Stopwatch.GetTimestamp() : 0;

var actionContext = new ActionContext(httpContext, routeData, actionDescriptor);
if (_actionContextAccessor != null)
{
_actionContextAccessor.ActionContext = actionContext;
}

var invoker = _actionInvokerFactory.CreateInvoker(actionContext);
if (invoker == null)
{
throw new InvalidOperationException(
Resources.FormatActionInvokerFactory_CouldNotCreateInvoker(
actionDescriptor.DisplayName));
}

await invoker.InvokeAsync();

_logger.ExecutedAction(actionDescriptor, startTimestamp);
}
_actionContextAccessor.ActionContext = actionContext;
}
finally

var invoker = _actionInvokerFactory.CreateInvoker(actionContext);
if (invoker == null)
{
_diagnosticSource.AfterAction(actionDescriptor, httpContext, routeData);
throw new InvalidOperationException(
Resources.FormatActionInvokerFactory_CouldNotCreateInvoker(
actionDescriptor.DisplayName));
}

context.Handler = async (c) => await invoker.InvokeAsync();
return TaskCache.CompletedTask;
}
}
}
76 changes: 0 additions & 76 deletions test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/MvcRouteHandlerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,37 +19,6 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure
{
public class MvcRouteHandlerTests
{
[Fact]
public async Task RouteHandler_Success_LogsCorrectValues()
{
// Arrange
var sink = new TestSink();
var loggerFactory = new TestLoggerFactory(sink, enabled: true);

var displayName = "A.B.C";
var actionDescriptor = new Mock<ActionDescriptor>();
actionDescriptor
.SetupGet(ad => ad.DisplayName)
.Returns(displayName);

var context = CreateRouteContext();

var handler = CreateMvcRouteHandler(actionDescriptor: actionDescriptor.Object, loggerFactory: loggerFactory);
await handler.RouteAsync(context);

// Act
await context.Handler(context.HttpContext);

// Assert
Assert.Single(sink.Scopes);
Assert.Equal(displayName, sink.Scopes[0].Scope?.ToString());

Assert.Equal(2, sink.Writes.Count);
Assert.Equal($"Executing action {displayName}", sink.Writes[0].State?.ToString());
// This message has the execution time embedded, which we don't want to verify.
Assert.StartsWith($"Executed action {displayName} ", sink.Writes[1].State?.ToString());
}

[Fact]
public async Task RouteAsync_FailOnNoAction_LogsCorrectValues()
{
Expand Down Expand Up @@ -112,51 +81,6 @@ public async Task RouteHandler_RemovesRouteGroupFromRouteValues()
Assert.False(context.RouteData.Values.ContainsKey(TreeRouter.RouteGroupKey));
}

[Fact]
public async Task RouteHandler_WritesDiagnostic_ActionSelected()
{
// Arrange
var listener = new TestDiagnosticListener();

var context = CreateRouteContext();
context.RouteData.Values.Add("tag", "value");

var handler = CreateMvcRouteHandler(diagnosticListener: listener);
await handler.RouteAsync(context);

// Act
await context.Handler(context.HttpContext);

// Assert
Assert.NotNull(listener.BeforeAction?.ActionDescriptor);
Assert.NotNull(listener.BeforeAction?.HttpContext);

var routeValues = listener.BeforeAction?.RouteData?.Values;
Assert.NotNull(routeValues);

Assert.Equal(1, routeValues.Count);
Assert.Contains(routeValues, kvp => kvp.Key == "tag" && string.Equals(kvp.Value, "value"));
}

[Fact]
public async Task RouteHandler_WritesDiagnostic_ActionInvoked()
{
// Arrange
var listener = new TestDiagnosticListener();

var context = CreateRouteContext();

var handler = CreateMvcRouteHandler(diagnosticListener: listener);
await handler.RouteAsync(context);

// Act
await context.Handler(context.HttpContext);

// Assert
Assert.NotNull(listener.AfterAction?.ActionDescriptor);
Assert.NotNull(listener.AfterAction?.HttpContext);
}

private MvcRouteHandler CreateMvcRouteHandler(
ActionDescriptor actionDescriptor = null,
IActionSelector actionSelector = null,
Expand Down
Loading

0 comments on commit 4cef233

Please sign in to comment.