From cc85c5a29708c380e25f4230fa4bb82363aff3cd Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Tue, 19 Jan 2021 10:56:30 -0800 Subject: [PATCH 1/2] [Mvc] Support IAsyncDisposable in controllers, pages and view components * Adds async versions of Release to the relevant interfaces in different factories and activators. * The new methods offer a default interface implementation that will delegate to the syncrhonous version and just call dispose. * Our implementations will override the default implementation and handle disposing IAsyncDisposable implementations. * Our implementations will favor IAsyncDisposable over IDisposable when both interfaces are implemented. * Extenders will have to override the new methods included to support IAsyncDisposable instances. --- .../ControllerActivatorProvider.cs | 47 ++++++ .../Controllers/ControllerFactoryProvider.cs | 29 +++- .../Controllers/DefaultControllerActivator.cs | 22 +++ .../Controllers/DefaultControllerFactory.cs | 16 ++ .../src/Controllers/IControllerActivator.cs | 15 +- .../IControllerActivatorProvider.cs | 18 +- .../src/Controllers/IControllerFactory.cs | 13 ++ .../Controllers/IControllerFactoryProvider.cs | 18 +- .../Infrastructure/ControllerActionInvoker.cs | 6 +- .../ControllerActionInvokerCache.cs | 2 +- .../ControllerActionInvokerCacheEntry.cs | 5 +- .../src/Infrastructure/ResourceInvoker.cs | 154 ++++++++++-------- src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt | 5 + .../ControllerFactoryProviderTest.cs | 28 +++- .../DefaultControllerActivatorTest.cs | 76 +++++++++ .../DefaultControllerFactoryTest.cs | 29 +++- .../test/Filters/MiddlewareFilterTest.cs | 13 +- .../ControllerActionInvokerTest.cs | 2 +- src/Mvc/Mvc.Razor/src/RazorViewEngine.cs | 5 + .../src/IPageActivatorProvider.cs | 20 ++- .../src/IPageFactoryProvider.cs | 21 ++- .../src/IPageModelActivatorProvider.cs | 20 ++- .../src/IPageModelFactoryProvider.cs | 20 ++- .../DefaultPageActivatorProvider.cs | 51 +++++- .../DefaultPageFactoryProvider.cs | 15 +- .../DefaultPageModelActivatorProvider.cs | 46 +++++- .../DefaultPageModelFactoryProvider.cs | 20 ++- .../src/Infrastructure/PageActionInvoker.cs | 8 +- .../Infrastructure/PageActionInvokerCache.cs | 7 +- .../PageActionInvokerCacheEntry.cs | 10 +- .../src/PublicAPI.Unshipped.txt | 4 + .../DefaultPageActivatorProviderTest.cs | 114 ++++++++++++- .../DefaultPageFactoryProviderTest.cs | 112 ++++++++++++- .../DefaultPageModelActivatorProviderTest.cs | 115 ++++++++++++- .../DefaultPageModelFactoryProviderTest.cs | 25 ++- .../PageActionInvokerProviderTest.cs | 13 +- .../Infrastructure/PageActionInvokerTest.cs | 4 +- .../src/PublicAPI.Unshipped.txt | 3 + .../DefaultViewComponentActivator.cs | 22 +++ .../DefaultViewComponentFactory.cs | 19 ++- .../DefaultViewComponentInvoker.cs | 43 +++-- .../ViewComponents/IViewComponentActivator.cs | 18 +- .../ViewComponents/IViewComponentFactory.cs | 16 +- .../src/ViewEngines/CompositeViewEngine.cs | 6 +- .../DefaultViewComponentActivatorTests.cs | 78 +++++++++ .../DefaultViewComponentFactoryTest.cs | 92 ++++++++++- .../Mvc.FunctionalTests/AsyncDisposalTest.cs | 68 ++++++++ .../Infrastructure/MvcTestFixture.cs | 1 + .../PageAsyncDisposalTest.cs | 44 +++++ .../Controllers/AsyncDisposableController.cs | 51 ++++++ .../RazorPagesWebSite/AsyncDisposable.cshtml | 13 ++ .../RazorPagesWebSite/PageTestDisposeAsync.cs | 12 ++ 52 files changed, 1469 insertions(+), 145 deletions(-) create mode 100644 src/Mvc/test/Mvc.FunctionalTests/AsyncDisposalTest.cs create mode 100644 src/Mvc/test/Mvc.FunctionalTests/PageAsyncDisposalTest.cs create mode 100644 src/Mvc/test/WebSites/BasicWebSite/Controllers/AsyncDisposableController.cs create mode 100644 src/Mvc/test/WebSites/RazorPagesWebSite/AsyncDisposable.cshtml create mode 100644 src/Mvc/test/WebSites/RazorPagesWebSite/PageTestDisposeAsync.cs diff --git a/src/Mvc/Mvc.Core/src/Controllers/ControllerActivatorProvider.cs b/src/Mvc/Mvc.Core/src/Controllers/ControllerActivatorProvider.cs index 61e1ad137e18..0e05725d55c5 100644 --- a/src/Mvc/Mvc.Core/src/Controllers/ControllerActivatorProvider.cs +++ b/src/Mvc/Mvc.Core/src/Controllers/ControllerActivatorProvider.cs @@ -3,6 +3,7 @@ using System; using System.Reflection; +using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Core; using Microsoft.Extensions.DependencyInjection; @@ -14,8 +15,11 @@ namespace Microsoft.AspNetCore.Mvc.Controllers public class ControllerActivatorProvider : IControllerActivatorProvider { private static readonly Action _dispose = Dispose; + private static readonly Func _disposeAsync = DisposeAsync; + private static readonly Func _syncDisposeAsync = SyncDisposeAsync; private readonly Func _controllerActivatorCreate; private readonly Action _controllerActivatorRelease; + private readonly Func _controllerActivatorReleaseAsync; /// /// Initializes a new instance of . @@ -33,6 +37,7 @@ public ControllerActivatorProvider(IControllerActivator controllerActivator) { _controllerActivatorCreate = controllerActivator.Create; _controllerActivatorRelease = controllerActivator.Release; + _controllerActivatorReleaseAsync = controllerActivator.ReleaseAsync; } } @@ -83,6 +88,32 @@ public Action CreateReleaser(ControllerActionDescript return null; } + /// + public Func CreateAsyncReleaser(ControllerActionDescriptor descriptor) + { + if (descriptor == null) + { + throw new ArgumentNullException(nameof(descriptor)); + } + + if (_controllerActivatorReleaseAsync != null) + { + return _controllerActivatorReleaseAsync; + } + + if (typeof(IAsyncDisposable).GetTypeInfo().IsAssignableFrom(descriptor.ControllerTypeInfo)) + { + return _disposeAsync; + } + + if (typeof(IDisposable).GetTypeInfo().IsAssignableFrom(descriptor.ControllerTypeInfo)) + { + return _syncDisposeAsync; + } + + return null; + } + private static void Dispose(ControllerContext context, object controller) { if (controller == null) @@ -92,5 +123,21 @@ private static void Dispose(ControllerContext context, object controller) ((IDisposable)controller).Dispose(); } + + private static ValueTask DisposeAsync(ControllerContext context, object controller) + { + if (controller == null) + { + throw new ArgumentNullException(nameof(controller)); + } + + return ((IAsyncDisposable)controller).DisposeAsync(); + } + + private static ValueTask SyncDisposeAsync(ControllerContext context, object controller) + { + Dispose(context, controller); + return default; + } } } diff --git a/src/Mvc/Mvc.Core/src/Controllers/ControllerFactoryProvider.cs b/src/Mvc/Mvc.Core/src/Controllers/ControllerFactoryProvider.cs index 5c0ee050885f..db4e6db546bf 100644 --- a/src/Mvc/Mvc.Core/src/Controllers/ControllerFactoryProvider.cs +++ b/src/Mvc/Mvc.Core/src/Controllers/ControllerFactoryProvider.cs @@ -1,9 +1,10 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Core; namespace Microsoft.AspNetCore.Mvc.Controllers @@ -13,6 +14,7 @@ internal class ControllerFactoryProvider : IControllerFactoryProvider private readonly IControllerActivatorProvider _activatorProvider; private readonly Func _factoryCreateController; private readonly Action _factoryReleaseController; + private readonly Func _factoryReleaseControllerAsync; private readonly IControllerPropertyActivator[] _propertyActivators; public ControllerFactoryProvider( @@ -37,6 +39,7 @@ public ControllerFactoryProvider( { _factoryCreateController = controllerFactory.CreateController; _factoryReleaseController = controllerFactory.ReleaseController; + _factoryReleaseControllerAsync = controllerFactory.ReleaseControllerAsync; } _propertyActivators = propertyActivators.ToArray(); @@ -104,6 +107,30 @@ public Action CreateControllerReleaser(ControllerActi return _activatorProvider.CreateReleaser(descriptor); } + public Func CreateAsyncControllerReleaser(ControllerActionDescriptor descriptor) + { + if (descriptor == null) + { + throw new ArgumentNullException(nameof(descriptor)); + } + + var controllerType = descriptor.ControllerTypeInfo?.AsType(); + if (controllerType == null) + { + throw new ArgumentException(Resources.FormatPropertyOfTypeCannotBeNull( + nameof(descriptor.ControllerTypeInfo), + nameof(descriptor)), + nameof(descriptor)); + } + + if (_factoryReleaseControllerAsync != null) + { + return _factoryReleaseControllerAsync; + } + + return _activatorProvider.CreateAsyncReleaser(descriptor); + } + private Action[] GetPropertiesToActivate(ControllerActionDescriptor actionDescriptor) { var propertyActivators = new Action[_propertyActivators.Length]; diff --git a/src/Mvc/Mvc.Core/src/Controllers/DefaultControllerActivator.cs b/src/Mvc/Mvc.Core/src/Controllers/DefaultControllerActivator.cs index 9d40ddf8ef81..013c4eca7853 100644 --- a/src/Mvc/Mvc.Core/src/Controllers/DefaultControllerActivator.cs +++ b/src/Mvc/Mvc.Core/src/Controllers/DefaultControllerActivator.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Core; using Microsoft.AspNetCore.Mvc.Infrastructure; @@ -74,5 +75,26 @@ public void Release(ControllerContext context, object controller) disposable.Dispose(); } } + + public ValueTask ReleaseAsync(ControllerContext context, object controller) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (controller == null) + { + throw new ArgumentNullException(nameof(controller)); + } + + if (controller is IAsyncDisposable asyncDisposable) + { + return asyncDisposable.DisposeAsync(); + } + + Release(context, controller); + return default; + } } } diff --git a/src/Mvc/Mvc.Core/src/Controllers/DefaultControllerFactory.cs b/src/Mvc/Mvc.Core/src/Controllers/DefaultControllerFactory.cs index 065b8ebc5c9c..b520d4871a72 100644 --- a/src/Mvc/Mvc.Core/src/Controllers/DefaultControllerFactory.cs +++ b/src/Mvc/Mvc.Core/src/Controllers/DefaultControllerFactory.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Core; namespace Microsoft.AspNetCore.Mvc.Controllers @@ -83,5 +84,20 @@ public void ReleaseController(ControllerContext context, object controller) _controllerActivator.Release(context, controller); } + + public ValueTask ReleaseControllerAsync(ControllerContext context, object controller) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (controller == null) + { + throw new ArgumentNullException(nameof(controller)); + } + + return _controllerActivator.ReleaseAsync(context, controller); + } } } diff --git a/src/Mvc/Mvc.Core/src/Controllers/IControllerActivator.cs b/src/Mvc/Mvc.Core/src/Controllers/IControllerActivator.cs index 87a4926be678..0fb0feacc169 100644 --- a/src/Mvc/Mvc.Core/src/Controllers/IControllerActivator.cs +++ b/src/Mvc/Mvc.Core/src/Controllers/IControllerActivator.cs @@ -1,6 +1,8 @@ // 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.Threading.Tasks; + namespace Microsoft.AspNetCore.Mvc.Controllers { /// @@ -20,5 +22,16 @@ public interface IControllerActivator /// The for the executing action. /// The controller to release. void Release(ControllerContext context, object controller); + + /// + /// Releases a controller asynchronously. + /// + /// The for the executing action. + /// The controller to release. + ValueTask ReleaseAsync(ControllerContext context, object controller) + { + Release(context, controller); + return default; + } } -} \ No newline at end of file +} diff --git a/src/Mvc/Mvc.Core/src/Controllers/IControllerActivatorProvider.cs b/src/Mvc/Mvc.Core/src/Controllers/IControllerActivatorProvider.cs index 0ff5d2b13474..ee528b0168bf 100644 --- a/src/Mvc/Mvc.Core/src/Controllers/IControllerActivatorProvider.cs +++ b/src/Mvc/Mvc.Core/src/Controllers/IControllerActivatorProvider.cs @@ -1,7 +1,8 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Threading.Tasks; namespace Microsoft.AspNetCore.Mvc.Controllers { @@ -23,5 +24,20 @@ public interface IControllerActivatorProvider /// The . /// The delegate used to dispose the activated controller. Action CreateReleaser(ControllerActionDescriptor descriptor); + + /// + /// Creates an that releases a controller. + /// + /// The . + /// The delegate used to dispose the activated controller. + Func CreateAsyncReleaser(ControllerActionDescriptor descriptor) + { + var releaser = CreateReleaser(descriptor); + return (context, controller) => + { + releaser(context, controller); + return default; + }; + } } } diff --git a/src/Mvc/Mvc.Core/src/Controllers/IControllerFactory.cs b/src/Mvc/Mvc.Core/src/Controllers/IControllerFactory.cs index ae66e446a7bb..1fc5897921b6 100644 --- a/src/Mvc/Mvc.Core/src/Controllers/IControllerFactory.cs +++ b/src/Mvc/Mvc.Core/src/Controllers/IControllerFactory.cs @@ -1,6 +1,8 @@ // 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.Threading.Tasks; + namespace Microsoft.AspNetCore.Mvc.Controllers { /// @@ -21,5 +23,16 @@ public interface IControllerFactory /// for the executing action. /// The controller. void ReleaseController(ControllerContext context, object controller); + + /// + /// Releases a controller instance asynchronously. + /// + /// for the executing action. + /// The controller. + ValueTask ReleaseControllerAsync(ControllerContext context, object controller) + { + ReleaseController(context, controller); + return default; + } } } diff --git a/src/Mvc/Mvc.Core/src/Controllers/IControllerFactoryProvider.cs b/src/Mvc/Mvc.Core/src/Controllers/IControllerFactoryProvider.cs index 4483047d565a..32f5bac53b96 100644 --- a/src/Mvc/Mvc.Core/src/Controllers/IControllerFactoryProvider.cs +++ b/src/Mvc/Mvc.Core/src/Controllers/IControllerFactoryProvider.cs @@ -1,7 +1,8 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Threading.Tasks; namespace Microsoft.AspNetCore.Mvc.Controllers { @@ -23,5 +24,20 @@ public interface IControllerFactoryProvider /// The . /// The delegate used to release the created controller. Action CreateControllerReleaser(ControllerActionDescriptor descriptor); + + /// + /// Releases a controller asynchronously. + /// + /// The . + /// The delegate used to release the created controller asynchronously. + Func CreateAsyncControllerReleaser(ControllerActionDescriptor descriptor) + { + var releaser = CreateControllerReleaser(descriptor); + return (context, controller) => + { + releaser(context, controller); + return default; + }; + } } } diff --git a/src/Mvc/Mvc.Core/src/Infrastructure/ControllerActionInvoker.cs b/src/Mvc/Mvc.Core/src/Infrastructure/ControllerActionInvoker.cs index 8759d78d8834..b1d73b334738 100644 --- a/src/Mvc/Mvc.Core/src/Infrastructure/ControllerActionInvoker.cs +++ b/src/Mvc/Mvc.Core/src/Infrastructure/ControllerActionInvoker.cs @@ -48,12 +48,14 @@ internal ControllerActionInvoker( // Internal for testing internal ControllerContext ControllerContext => _controllerContext; - protected override void ReleaseResources() + protected override ValueTask ReleaseResources() { if (_instance != null && _cacheEntry.ControllerReleaser != null) { - _cacheEntry.ControllerReleaser(_controllerContext, _instance); + return _cacheEntry.ControllerReleaser(_controllerContext, _instance); } + + return default; } private Task Next(ref State next, ref Scope scope, ref object? state, ref bool isCompleted) diff --git a/src/Mvc/Mvc.Core/src/Infrastructure/ControllerActionInvokerCache.cs b/src/Mvc/Mvc.Core/src/Infrastructure/ControllerActionInvokerCache.cs index a273470eddd5..74bb34db4e62 100644 --- a/src/Mvc/Mvc.Core/src/Infrastructure/ControllerActionInvokerCache.cs +++ b/src/Mvc/Mvc.Core/src/Infrastructure/ControllerActionInvokerCache.cs @@ -61,7 +61,7 @@ public ControllerActionInvokerCache( parameterDefaultValues); var controllerFactory = _controllerFactoryProvider.CreateControllerFactory(actionDescriptor); - var controllerReleaser = _controllerFactoryProvider.CreateControllerReleaser(actionDescriptor); + var controllerReleaser = _controllerFactoryProvider.CreateAsyncControllerReleaser(actionDescriptor); var propertyBinderFactory = ControllerBinderDelegateProvider.CreateBinderDelegate( _parameterBinder, _modelBinderFactory, diff --git a/src/Mvc/Mvc.Core/src/Infrastructure/ControllerActionInvokerCacheEntry.cs b/src/Mvc/Mvc.Core/src/Infrastructure/ControllerActionInvokerCacheEntry.cs index 8b471215b35f..3b10b6a9adc3 100644 --- a/src/Mvc/Mvc.Core/src/Infrastructure/ControllerActionInvokerCacheEntry.cs +++ b/src/Mvc/Mvc.Core/src/Infrastructure/ControllerActionInvokerCacheEntry.cs @@ -4,6 +4,7 @@ #nullable enable using System; +using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.Extensions.Internal; @@ -15,7 +16,7 @@ internal class ControllerActionInvokerCacheEntry internal ControllerActionInvokerCacheEntry( FilterItem[] cachedFilters, Func controllerFactory, - Action controllerReleaser, + Func controllerReleaser, ControllerBinderDelegate controllerBinderDelegate, ObjectMethodExecutor objectMethodExecutor, ActionMethodExecutor actionMethodExecutor) @@ -32,7 +33,7 @@ internal ControllerActionInvokerCacheEntry( public Func ControllerFactory { get; } - public Action ControllerReleaser { get; } + public Func ControllerReleaser { get; } public ControllerBinderDelegate ControllerBinderDelegate { get; } diff --git a/src/Mvc/Mvc.Core/src/Infrastructure/ResourceInvoker.cs b/src/Mvc/Mvc.Core/src/Infrastructure/ResourceInvoker.cs index 9f90b682d2d8..02a14bfb60b0 100644 --- a/src/Mvc/Mvc.Core/src/Infrastructure/ResourceInvoker.cs +++ b/src/Mvc/Mvc.Core/src/Infrastructure/ResourceInvoker.cs @@ -83,59 +83,17 @@ public virtual Task InvokeAsync() return Awaited(this, task, scope); } - Exception? releaseException = null; - try - { - ReleaseResources(); - } - catch (Exception exception) - { - releaseException = exception; - } - - Exception? scopeException = null; - try - { - scope?.Dispose(); - } - catch (Exception exception) - { - scopeException = exception; - } - - if (releaseException == null && scopeException == null) - { - return Task.CompletedTask; - } - else if (releaseException != null && scopeException != null) - { - return Task.FromException(new AggregateException(releaseException, scopeException)); - } - else if (releaseException != null) - { - return Task.FromException(releaseException); - } - else - { - return Task.FromException(scopeException!); - } + return ReleaseResourcesCore(scope); static async Task Awaited(ResourceInvoker invoker, Task task, IDisposable? scope) { try { - try - { - await task; - } - finally - { - invoker.ReleaseResources(); - } + await task; } finally { - scope?.Dispose(); + await invoker.ReleaseResourcesCore(scope); } } @@ -143,7 +101,6 @@ static async Task Logged(ResourceInvoker invoker) { var actionContext = invoker._actionContext; invoker._actionContextAccessor.ActionContext = actionContext; - try { var logger = invoker._logger; @@ -153,28 +110,27 @@ static async Task Logged(ResourceInvoker invoker) actionContext.HttpContext, actionContext.RouteData); - using (logger.ActionScope(actionContext.ActionDescriptor)) - { - logger.ExecutingAction(actionContext.ActionDescriptor); + var actionScope = logger.ActionScope(actionContext.ActionDescriptor); - var filters = invoker._filters; - logger.AuthorizationFiltersExecutionPlan(filters); - logger.ResourceFiltersExecutionPlan(filters); - logger.ActionFiltersExecutionPlan(filters); - logger.ExceptionFiltersExecutionPlan(filters); - logger.ResultFiltersExecutionPlan(filters); + logger.ExecutingAction(actionContext.ActionDescriptor); - var stopwatch = ValueStopwatch.StartNew(); + var filters = invoker._filters; + logger.AuthorizationFiltersExecutionPlan(filters); + logger.ResourceFiltersExecutionPlan(filters); + logger.ActionFiltersExecutionPlan(filters); + logger.ExceptionFiltersExecutionPlan(filters); + logger.ResultFiltersExecutionPlan(filters); - try - { - await invoker.InvokeFilterPipelineAsync(); - } - finally - { - invoker.ReleaseResources(); - logger.ExecutedAction(actionContext.ActionDescriptor, stopwatch.GetElapsedTime()); - } + var stopwatch = ValueStopwatch.StartNew(); + + try + { + await invoker.InvokeFilterPipelineAsync(); + } + finally + { + await invoker.ReleaseResourcesCore(actionScope); + logger.ExecutedAction(actionContext.ActionDescriptor, stopwatch.GetElapsedTime()); } } finally @@ -187,11 +143,77 @@ static async Task Logged(ResourceInvoker invoker) } } + internal Task ReleaseResourcesCore(IDisposable? scope) + { + Exception? releaseException = null; + ValueTask releaseResult = default; + try + { + releaseResult = ReleaseResources(); + } + catch (Exception exception) + { + releaseException = exception; + } + + if (releaseException == null && !releaseResult.IsCompletedSuccessfully) + { + return HandleAsyncReleaseErrors(releaseResult, scope); + } + + return HandleReleaseErrors(scope, releaseException); + + static async Task HandleAsyncReleaseErrors(ValueTask releaseResult, IDisposable? scope) + { + Exception? releaseException = null; + try + { + await releaseResult; + } + catch (Exception exception) + { + releaseException = exception; + } + + await HandleReleaseErrors(scope, releaseException); + } + + static Task HandleReleaseErrors(IDisposable? scope, Exception? releaseException) + { + Exception? scopeException = null; + try + { + scope?.Dispose(); + } + catch (Exception exception) + { + scopeException = exception; + } + + if (releaseException == null && scopeException == null) + { + return Task.CompletedTask; + } + else if (releaseException != null && scopeException != null) + { + return Task.FromException(new AggregateException(releaseException, scopeException)); + } + else if (releaseException != null) + { + return Task.FromException(releaseException); + } + else + { + return Task.FromException(scopeException!); + } + } + } + /// /// In derived types, releases resources such as controller, model, or page instances created as /// part of invoking the inner pipeline. /// - protected abstract void ReleaseResources(); + protected abstract ValueTask ReleaseResources(); private Task InvokeFilterPipelineAsync() { diff --git a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt index e69de29bb2d1..ca51e9c33144 100644 --- a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt +++ b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt @@ -0,0 +1,5 @@ +~Microsoft.AspNetCore.Mvc.Controllers.ControllerActivatorProvider.CreateAsyncReleaser(Microsoft.AspNetCore.Mvc.Controllers.ControllerActionDescriptor descriptor) -> System.Func +~Microsoft.AspNetCore.Mvc.Controllers.IControllerActivator.ReleaseAsync(Microsoft.AspNetCore.Mvc.ControllerContext context, object controller) -> System.Threading.Tasks.ValueTask +~Microsoft.AspNetCore.Mvc.Controllers.IControllerActivatorProvider.CreateAsyncReleaser(Microsoft.AspNetCore.Mvc.Controllers.ControllerActionDescriptor descriptor) -> System.Func +~Microsoft.AspNetCore.Mvc.Controllers.IControllerFactory.ReleaseControllerAsync(Microsoft.AspNetCore.Mvc.ControllerContext context, object controller) -> System.Threading.Tasks.ValueTask +~Microsoft.AspNetCore.Mvc.Controllers.IControllerFactoryProvider.CreateAsyncControllerReleaser(Microsoft.AspNetCore.Mvc.Controllers.ControllerActionDescriptor descriptor) -> System.Func \ No newline at end of file diff --git a/src/Mvc/Mvc.Core/test/Controllers/ControllerFactoryProviderTest.cs b/src/Mvc/Mvc.Core/test/Controllers/ControllerFactoryProviderTest.cs index 8e4553326370..fc86fc201e78 100644 --- a/src/Mvc/Mvc.Core/test/Controllers/ControllerFactoryProviderTest.cs +++ b/src/Mvc/Mvc.Core/test/Controllers/ControllerFactoryProviderTest.cs @@ -1,9 +1,10 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; using System.Linq; using System.Reflection; +using System.Threading.Tasks; using Moq; using Xunit; @@ -63,6 +64,31 @@ public void CreateControllerReleaser_InvokesIControllerFactory_IfItIsNotDefaultC factory.Verify(); } + [Fact] + public async Task CreateAsyncControllerReleaser_InvokesIControllerFactory_IfItIsNotDefaultControllerFactoryAsync() + { + // Arrange + var controller = new object(); + var factory = new Mock(); + factory.Setup(f => f.ReleaseControllerAsync(It.IsAny(), controller)) + .Verifiable(); + var provider = new ControllerFactoryProvider( + Mock.Of(), + factory.Object, + Enumerable.Empty()); + var descriptor = new ControllerActionDescriptor + { + ControllerTypeInfo = typeof(object).GetTypeInfo(), + }; + + // Act + var releaser = provider.CreateAsyncControllerReleaser(descriptor); + await releaser(new ControllerContext(), controller); + + // Assert + factory.Verify(); + } + [Fact] public void CreateControllerFactory_UsesControllerActivatorAndPropertyActivator() { diff --git a/src/Mvc/Mvc.Core/test/Controllers/DefaultControllerActivatorTest.cs b/src/Mvc/Mvc.Core/test/Controllers/DefaultControllerActivatorTest.cs index 01bbb8af062a..ea99a7e6224b 100644 --- a/src/Mvc/Mvc.Core/test/Controllers/DefaultControllerActivatorTest.cs +++ b/src/Mvc/Mvc.Core/test/Controllers/DefaultControllerActivatorTest.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Reflection; +using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.ModelBinding; @@ -60,6 +61,49 @@ public void Release_DisposesController_IfDisposable() Assert.True(controller.Disposed); } + [Fact] + public async Task ReleaseAsync_AsynchronouslyDisposesController_IfAsyncDisposableAsync() + { + // Arrange + var controller = new MyAsyncDisposableController(); + var activator = new DefaultControllerActivator(Mock.Of()); + + // Act + await activator.ReleaseAsync(new ControllerContext(), controller); + + // Assert + Assert.True(controller.Disposed); + } + + [Fact] + public async Task ReleaseAsync_SynchronouslyDisposesController_IfDisposableAsync() + { + // Arrange + var controller = new MyController(); + var activator = new DefaultControllerActivator(Mock.Of()); + + // Act + await activator.ReleaseAsync(new ControllerContext(), controller); + + // Assert + Assert.True(controller.Disposed); + } + + [Fact] + public async Task ReleaseAsync_SynchronouslyDisposesController_PrefersDisposeAsyncOverDispose() + { + // Arrange + var controller = new MyDisposableAndAsyncDisposableController(); + var activator = new DefaultControllerActivator(Mock.Of()); + + // Act + await activator.ReleaseAsync(new ControllerContext(), controller); + + // Assert + Assert.False(controller.SyncDisposed); + Assert.True(controller.AsyncDisposed); + } + [Fact] public void DefaultControllerActivator_ReleasesNonIDisposableController() { @@ -156,5 +200,37 @@ public void Dispose() Disposed = true; } } + + private class MyAsyncDisposableController : IAsyncDisposable + { + public bool Disposed { get; set; } + + public void Dispose() + { + } + + public ValueTask DisposeAsync() + { + Disposed = true; + return default; + } + } + + private class MyDisposableAndAsyncDisposableController : IDisposable, IAsyncDisposable + { + public bool AsyncDisposed { get; set; } + public bool SyncDisposed { get; set; } + + public void Dispose() + { + SyncDisposed = true; + } + + public ValueTask DisposeAsync() + { + AsyncDisposed = true; + return default; + } + } } } diff --git a/src/Mvc/Mvc.Core/test/Controllers/DefaultControllerFactoryTest.cs b/src/Mvc/Mvc.Core/test/Controllers/DefaultControllerFactoryTest.cs index 96de5c689f82..060ad2e7ba2f 100644 --- a/src/Mvc/Mvc.Core/test/Controllers/DefaultControllerFactoryTest.cs +++ b/src/Mvc/Mvc.Core/test/Controllers/DefaultControllerFactoryTest.cs @@ -3,11 +3,11 @@ using System; using System.Reflection; +using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; -using Microsoft.AspNetCore.Testing; using Moq; using Xunit; @@ -202,6 +202,22 @@ public void DefaultControllerFactory_DelegatesDisposalToControllerActivator() activatorMock.Verify(); } + [Fact] + public async Task DefaultControllerFactory_DelegatesAsyncDisposalToControllerActivatorAsync() + { + // Arrange + var activatorMock = new Mock(); + activatorMock.Setup(s => s.Release(It.IsAny(), It.IsAny())); + + var factory = CreateControllerFactory(activatorMock.Object); + var controller = new MyAsyncDisposableController(); + + // Act + Assert + await factory.ReleaseControllerAsync(new ControllerContext(), controller); + + activatorMock.Verify(); + } + private IServiceProvider GetServices() { var metadataProvider = new EmptyModelMetadataProvider(); @@ -267,6 +283,17 @@ public void Dispose() } } + private class MyAsyncDisposableController : IAsyncDisposable + { + public bool Disposed { get; set; } + + public ValueTask DisposeAsync() + { + Disposed = true; + return default; + } + } + private class ControllerThatCannotBeActivated { public ControllerThatCannotBeActivated(TestService service) diff --git a/src/Mvc/Mvc.Core/test/Filters/MiddlewareFilterTest.cs b/src/Mvc/Mvc.Core/test/Filters/MiddlewareFilterTest.cs index d054f8590a41..5b02a3be9769 100644 --- a/src/Mvc/Mvc.Core/test/Filters/MiddlewareFilterTest.cs +++ b/src/Mvc/Mvc.Core/test/Filters/MiddlewareFilterTest.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -372,6 +372,15 @@ public void ReleaseController(ControllerContext context, object controller) ReleaseCalled = true; } + public ValueTask ReleaseControllerAsync(ControllerContext context, object controller) + { + Assert.NotNull(controller); + Assert.Same(_controller, controller); + ReleaseCalled = true; + + return default; + } + public void Verify() { if (CreateCalled && !ReleaseCalled) @@ -441,7 +450,7 @@ private static ControllerActionInvokerCacheEntry CreateCacheEntry( return new ControllerActionInvokerCacheEntry( new FilterItem[0], controllerFactory.CreateController, - controllerFactory.ReleaseController, + controllerFactory.ReleaseControllerAsync, null, objectMethodExecutor, ActionMethodExecutor.GetExecutor(objectMethodExecutor)); diff --git a/src/Mvc/Mvc.Core/test/Infrastructure/ControllerActionInvokerTest.cs b/src/Mvc/Mvc.Core/test/Infrastructure/ControllerActionInvokerTest.cs index 2fe4f3c9b425..9ab07f66a1e9 100644 --- a/src/Mvc/Mvc.Core/test/Infrastructure/ControllerActionInvokerTest.cs +++ b/src/Mvc/Mvc.Core/test/Infrastructure/ControllerActionInvokerTest.cs @@ -1336,7 +1336,7 @@ public async Task Invoke_UsesDefaultValuesIfNotBound() var cacheEntry = new ControllerActionInvokerCacheEntry( new FilterItem[0], _ => new TestController(), - (_, __) => { }, + (_, __) => default, (_, __, ___) => Task.CompletedTask, objectMethodExecutor, controllerMethodExecutor); diff --git a/src/Mvc/Mvc.Razor/src/RazorViewEngine.cs b/src/Mvc/Mvc.Razor/src/RazorViewEngine.cs index bfa366575c7b..1df8f759ad30 100644 --- a/src/Mvc/Mvc.Razor/src/RazorViewEngine.cs +++ b/src/Mvc/Mvc.Razor/src/RazorViewEngine.cs @@ -484,6 +484,11 @@ private ViewEngineResult CreateViewEngineResult(ViewLocationCacheResult result, } var view = new RazorView(this, _pageActivator, viewStarts, page, _htmlEncoder, _diagnosticListener); + if (view is IAsyncDisposable) + { + throw new InvalidOperationException("Async disposable views are not supported."); + } + return ViewEngineResult.Found(viewName, view); } diff --git a/src/Mvc/Mvc.RazorPages/src/IPageActivatorProvider.cs b/src/Mvc/Mvc.RazorPages/src/IPageActivatorProvider.cs index 80578d5f1772..91485356ba2e 100644 --- a/src/Mvc/Mvc.RazorPages/src/IPageActivatorProvider.cs +++ b/src/Mvc/Mvc.RazorPages/src/IPageActivatorProvider.cs @@ -1,7 +1,8 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Rendering; namespace Microsoft.AspNetCore.Mvc.RazorPages @@ -24,5 +25,20 @@ public interface IPageActivatorProvider /// The . /// The delegate used to dispose the activated page. Action CreateReleaser(CompiledPageActionDescriptor descriptor); + + /// + /// Releases a Razor page asynchronously. + /// + /// The . + /// The delegate used to dispose the activated page asynchronously. + Func CreateAsyncReleaser(CompiledPageActionDescriptor descriptor) + { + var releaser = CreateReleaser(descriptor); + return (context, viewContext, page) => + { + releaser(context, viewContext, page); + return default; + }; + } } -} \ No newline at end of file +} diff --git a/src/Mvc/Mvc.RazorPages/src/IPageFactoryProvider.cs b/src/Mvc/Mvc.RazorPages/src/IPageFactoryProvider.cs index a3b366784c50..ae75927aa861 100644 --- a/src/Mvc/Mvc.RazorPages/src/IPageFactoryProvider.cs +++ b/src/Mvc/Mvc.RazorPages/src/IPageFactoryProvider.cs @@ -1,7 +1,8 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Rendering; namespace Microsoft.AspNetCore.Mvc.RazorPages @@ -24,5 +25,21 @@ public interface IPageFactoryProvider /// The . /// The delegate used to release the created page. Action CreatePageDisposer(CompiledPageActionDescriptor descriptor); + + /// + /// Releases a Razor page asynchronously. + /// + /// The . + /// The delegate used to release the created page asynchronously. + Func CreateAsyncPageDisposer(CompiledPageActionDescriptor descriptor) + { + var disposer = CreatePageDisposer(descriptor); + + return (context, viewContext, page) => + { + disposer(context, viewContext, page); + return default; + }; + } } -} \ No newline at end of file +} diff --git a/src/Mvc/Mvc.RazorPages/src/IPageModelActivatorProvider.cs b/src/Mvc/Mvc.RazorPages/src/IPageModelActivatorProvider.cs index 87891410870c..ee6107c0ecb7 100644 --- a/src/Mvc/Mvc.RazorPages/src/IPageModelActivatorProvider.cs +++ b/src/Mvc/Mvc.RazorPages/src/IPageModelActivatorProvider.cs @@ -1,7 +1,8 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Threading.Tasks; namespace Microsoft.AspNetCore.Mvc.RazorPages { @@ -23,5 +24,20 @@ public interface IPageModelActivatorProvider /// The . /// The delegate used to dispose the activated Razor Page model. Action CreateReleaser(CompiledPageActionDescriptor descriptor); + + /// + /// Releases a Razor Page model asynchronously. + /// + /// The . + /// The delegate used to dispose the activated Razor Page model asynchronously. + Func CreateAsyncReleaser(CompiledPageActionDescriptor descriptor) + { + var releaser = CreateReleaser(descriptor); + return (context, model) => + { + releaser(context, model); + return default; + }; + } } -} \ No newline at end of file +} diff --git a/src/Mvc/Mvc.RazorPages/src/IPageModelFactoryProvider.cs b/src/Mvc/Mvc.RazorPages/src/IPageModelFactoryProvider.cs index b99a7b1a2c3a..97998da7ad17 100644 --- a/src/Mvc/Mvc.RazorPages/src/IPageModelFactoryProvider.cs +++ b/src/Mvc/Mvc.RazorPages/src/IPageModelFactoryProvider.cs @@ -1,7 +1,8 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Threading.Tasks; namespace Microsoft.AspNetCore.Mvc.RazorPages { @@ -23,5 +24,20 @@ public interface IPageModelFactoryProvider /// The . /// The delegate used to release the created Razor Page model. Action CreateModelDisposer(CompiledPageActionDescriptor descriptor); + + /// + /// Releases a Razor Page model asynchronously. + /// + /// The . + /// The delegate used to release the created Razor Page model asynchronously. + Func CreateAsyncModelDisposer(CompiledPageActionDescriptor descriptor) + { + var releaser = CreateModelDisposer(descriptor); + return (context, model) => + { + releaser(context, model); + return default; + }; + } } -} \ No newline at end of file +} diff --git a/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageActivatorProvider.cs b/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageActivatorProvider.cs index 3d5c9ac4521d..3bf6302c7824 100644 --- a/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageActivatorProvider.cs +++ b/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageActivatorProvider.cs @@ -1,9 +1,10 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; using System.Linq.Expressions; using System.Reflection; +using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Rendering; namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure @@ -14,6 +15,8 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure internal class DefaultPageActivatorProvider : IPageActivatorProvider { private readonly Action _disposer = Dispose; + private readonly Func _asyncDisposer = AsyncDispose; + private readonly Func _syncAsyncDisposer = SyncAsyncDispose; /// public Func CreateActivator(CompiledPageActionDescriptor actionDescriptor) @@ -50,6 +53,26 @@ public Action CreateReleaser(CompiledPageActio return null; } + public Func CreateAsyncReleaser(CompiledPageActionDescriptor actionDescriptor) + { + if (actionDescriptor == null) + { + throw new ArgumentNullException(nameof(actionDescriptor)); + } + + if (typeof(IAsyncDisposable).GetTypeInfo().IsAssignableFrom(actionDescriptor.PageTypeInfo)) + { + return _asyncDisposer; + } + + if (typeof(IDisposable).GetTypeInfo().IsAssignableFrom(actionDescriptor.PageTypeInfo)) + { + return _syncAsyncDisposer; + } + + return null; + } + private static Func CreatePageFactory(Type pageTypeInfo) { var parameter1 = Expression.Parameter(typeof(PageContext), "pageContext"); @@ -84,5 +107,31 @@ private static void Dispose(PageContext context, ViewContext viewContext, object ((IDisposable)page).Dispose(); } + + private static ValueTask SyncAsyncDispose(PageContext context, ViewContext viewContext, object page) + { + Dispose(context, viewContext, page); + return default; + } + + private static ValueTask AsyncDispose(PageContext context, ViewContext viewContext, object page) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (viewContext == null) + { + throw new ArgumentNullException(nameof(viewContext)); + } + + if (page == null) + { + throw new ArgumentNullException(nameof(page)); + } + + return ((IAsyncDisposable)page).DisposeAsync(); + } } } diff --git a/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageFactoryProvider.cs b/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageFactoryProvider.cs index 607586c7ccc1..7526c2e6a4fe 100644 --- a/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageFactoryProvider.cs +++ b/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageFactoryProvider.cs @@ -1,10 +1,11 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; using System.Diagnostics; using System.Reflection; using System.Text.Encodings.Web; +using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.Razor; using Microsoft.AspNetCore.Mvc.Rendering; @@ -77,5 +78,15 @@ public Action CreatePageDisposer(CompiledPageA return _pageActivator.CreateReleaser(descriptor); } + + public Func CreateAsyncPageDisposer(CompiledPageActionDescriptor descriptor) + { + if (descriptor == null) + { + throw new ArgumentNullException(nameof(descriptor)); + } + + return _pageActivator.CreateAsyncReleaser(descriptor); + } } -} \ No newline at end of file +} diff --git a/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageModelActivatorProvider.cs b/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageModelActivatorProvider.cs index e66efcabb06f..edeeb72412a9 100644 --- a/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageModelActivatorProvider.cs +++ b/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageModelActivatorProvider.cs @@ -1,8 +1,9 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; using System.Reflection; +using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure @@ -13,6 +14,8 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure internal class DefaultPageModelActivatorProvider : IPageModelActivatorProvider { private readonly Action _disposer = Dispose; + private readonly Func _asyncDisposer = DisposeAsync; + private readonly Func _syncAsyncDisposer = SyncDisposeAsync; /// public virtual Func CreateActivator(CompiledPageActionDescriptor actionDescriptor) @@ -50,6 +53,26 @@ public virtual Action CreateReleaser(CompiledPageActionDesc return null; } + public virtual Func CreateAsyncReleaser(CompiledPageActionDescriptor actionDescriptor) + { + if (actionDescriptor == null) + { + throw new ArgumentNullException(nameof(actionDescriptor)); + } + + if (typeof(IAsyncDisposable).GetTypeInfo().IsAssignableFrom(actionDescriptor.ModelTypeInfo)) + { + return _asyncDisposer; + } + + if (typeof(IDisposable).GetTypeInfo().IsAssignableFrom(actionDescriptor.ModelTypeInfo)) + { + return _syncAsyncDisposer; + } + + return null; + } + private static void Dispose(PageContext context, object page) { if (context == null) @@ -64,5 +87,26 @@ private static void Dispose(PageContext context, object page) ((IDisposable)page).Dispose(); } + + private static ValueTask DisposeAsync(PageContext context, object page) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (page == null) + { + throw new ArgumentNullException(nameof(page)); + } + + return ((IAsyncDisposable)page).DisposeAsync(); + } + + private static ValueTask SyncDisposeAsync(PageContext context, object page) + { + Dispose(context, page); + return default; + } } } diff --git a/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageModelFactoryProvider.cs b/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageModelFactoryProvider.cs index 4b1eb168e8a9..ae10ed2ea973 100644 --- a/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageModelFactoryProvider.cs +++ b/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageModelFactoryProvider.cs @@ -1,8 +1,9 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; using System.Reflection; +using System.Threading.Tasks; using Microsoft.Extensions.Internal; namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure @@ -64,7 +65,22 @@ public Action CreateModelDisposer(CompiledPageActionDescrip return _modelActivator.CreateReleaser(descriptor); } + public Func CreateAsyncModelDisposer(CompiledPageActionDescriptor descriptor) + { + if (descriptor == null) + { + throw new ArgumentNullException(nameof(descriptor)); + } + + if (descriptor.ModelTypeInfo == null) + { + return null; + } + + return _modelActivator.CreateAsyncReleaser(descriptor); + } + private static PropertyActivator CreateActivateInfo(PropertyInfo property) => new PropertyActivator(property, pageContext => pageContext); } -} \ No newline at end of file +} diff --git a/src/Mvc/Mvc.RazorPages/src/Infrastructure/PageActionInvoker.cs b/src/Mvc/Mvc.RazorPages/src/Infrastructure/PageActionInvoker.cs index dc7aaf98e080..ebf69961aa1f 100644 --- a/src/Mvc/Mvc.RazorPages/src/Infrastructure/PageActionInvoker.cs +++ b/src/Mvc/Mvc.RazorPages/src/Infrastructure/PageActionInvoker.cs @@ -88,17 +88,19 @@ protected override async Task InvokeInnerFilterAsync() } } - protected override void ReleaseResources() + protected override ValueTask ReleaseResources() { if (_pageModel != null && CacheEntry.ReleaseModel != null) { - CacheEntry.ReleaseModel(_pageContext, _pageModel); + return CacheEntry.ReleaseModel(_pageContext, _pageModel); } if (_page != null && CacheEntry.ReleasePage != null) { - CacheEntry.ReleasePage(_pageContext, _viewContext, _page); + return CacheEntry.ReleasePage(_pageContext, _viewContext, _page); } + + return default; } protected override Task InvokeResultAsync(IActionResult result) diff --git a/src/Mvc/Mvc.RazorPages/src/Infrastructure/PageActionInvokerCache.cs b/src/Mvc/Mvc.RazorPages/src/Infrastructure/PageActionInvokerCache.cs index c5b154cd5886..0aa0daed4917 100644 --- a/src/Mvc/Mvc.RazorPages/src/Infrastructure/PageActionInvokerCache.cs +++ b/src/Mvc/Mvc.RazorPages/src/Infrastructure/PageActionInvokerCache.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; +using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.Razor; @@ -81,7 +82,7 @@ private PageActionInvokerCacheEntry CreateCacheEntry( var viewDataFactory = ViewDataDictionaryFactory.CreateFactory(compiledActionDescriptor.DeclaredModelTypeInfo); var pageFactory = _pageFactoryProvider.CreatePageFactory(compiledActionDescriptor); - var pageDisposer = _pageFactoryProvider.CreatePageDisposer(compiledActionDescriptor); + var pageDisposer = _pageFactoryProvider.CreateAsyncPageDisposer(compiledActionDescriptor); var propertyBinder = PageBinderFactory.CreatePropertyBinder( _parameterBinder, _modelMetadataProvider, @@ -89,11 +90,11 @@ private PageActionInvokerCacheEntry CreateCacheEntry( compiledActionDescriptor); Func modelFactory = null; - Action modelReleaser = null; + Func modelReleaser = null; if (compiledActionDescriptor.ModelTypeInfo != compiledActionDescriptor.PageTypeInfo) { modelFactory = _modelFactoryProvider.CreateModelFactory(compiledActionDescriptor); - modelReleaser = _modelFactoryProvider.CreateModelDisposer(compiledActionDescriptor); + modelReleaser = _modelFactoryProvider.CreateAsyncModelDisposer(compiledActionDescriptor); } var viewStartFactories = GetViewStartFactories(compiledActionDescriptor); diff --git a/src/Mvc/Mvc.RazorPages/src/Infrastructure/PageActionInvokerCacheEntry.cs b/src/Mvc/Mvc.RazorPages/src/Infrastructure/PageActionInvokerCacheEntry.cs index 9b964ff661cf..f2fad48c7c6f 100644 --- a/src/Mvc/Mvc.RazorPages/src/Infrastructure/PageActionInvokerCacheEntry.cs +++ b/src/Mvc/Mvc.RazorPages/src/Infrastructure/PageActionInvokerCacheEntry.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -18,9 +18,9 @@ public PageActionInvokerCacheEntry( CompiledPageActionDescriptor actionDescriptor, Func viewDataFactory, Func pageFactory, - Action releasePage, + Func releasePage, Func modelFactory, - Action releaseModel, + Func releaseModel, Func propertyBinder, PageHandlerExecutorDelegate[] handlerExecutors, PageHandlerBinderDelegate[] handlerBinders, @@ -47,14 +47,14 @@ public PageActionInvokerCacheEntry( /// /// The action invoked to release a page. This may be null. /// - public Action ReleasePage { get; } + public Func ReleasePage { get; } public Func ModelFactory { get; } /// /// The delegate invoked to release a model. This may be null. /// - public Action ReleaseModel { get; } + public Func ReleaseModel { get; } /// /// The delegate invoked to bind either the handler type (page or model). diff --git a/src/Mvc/Mvc.RazorPages/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.RazorPages/src/PublicAPI.Unshipped.txt index 7dc5c58110bf..9f2940be9bfa 100644 --- a/src/Mvc/Mvc.RazorPages/src/PublicAPI.Unshipped.txt +++ b/src/Mvc/Mvc.RazorPages/src/PublicAPI.Unshipped.txt @@ -1 +1,5 @@ #nullable enable +~Microsoft.AspNetCore.Mvc.RazorPages.IPageActivatorProvider.CreateAsyncReleaser(Microsoft.AspNetCore.Mvc.RazorPages.CompiledPageActionDescriptor descriptor) -> System.Func +~Microsoft.AspNetCore.Mvc.RazorPages.IPageFactoryProvider.CreateAsyncPageDisposer(Microsoft.AspNetCore.Mvc.RazorPages.CompiledPageActionDescriptor descriptor) -> System.Func +~Microsoft.AspNetCore.Mvc.RazorPages.IPageModelActivatorProvider.CreateAsyncReleaser(Microsoft.AspNetCore.Mvc.RazorPages.CompiledPageActionDescriptor descriptor) -> System.Func +~Microsoft.AspNetCore.Mvc.RazorPages.IPageModelFactoryProvider.CreateAsyncModelDisposer(Microsoft.AspNetCore.Mvc.RazorPages.CompiledPageActionDescriptor descriptor) -> System.Func diff --git a/src/Mvc/Mvc.RazorPages/test/Infrastructure/DefaultPageActivatorProviderTest.cs b/src/Mvc/Mvc.RazorPages/test/Infrastructure/DefaultPageActivatorProviderTest.cs index 4276f81ae904..db6aafa7fe73 100644 --- a/src/Mvc/Mvc.RazorPages/test/Infrastructure/DefaultPageActivatorProviderTest.cs +++ b/src/Mvc/Mvc.RazorPages/test/Infrastructure/DefaultPageActivatorProviderTest.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -87,6 +87,26 @@ public void CreateReleaser_ReturnsNullForPagesThatDoNotImplementDisposable(Type Assert.Null(releaser); } + [Theory] + [InlineData(typeof(TestPage))] + [InlineData(typeof(object))] + public void CreateAsyncReleaser_ReturnsNullForPagesThatDoNotImplementDisposable(Type pageType) + { + // Arrange + var context = new PageContext(); + var activator = new DefaultPageActivatorProvider(); + var page = new TestPage(); + + // Act + var releaser = activator.CreateAsyncReleaser(new CompiledPageActionDescriptor + { + PageTypeInfo = pageType.GetTypeInfo() + }); + + // Assert + Assert.Null(releaser); + } + [Fact] public void CreateReleaser_CreatesDelegateThatDisposesDisposableTypes() { @@ -108,6 +128,70 @@ public void CreateReleaser_CreatesDelegateThatDisposesDisposableTypes() Assert.True(page.Disposed); } + [Fact] + public void CreateAsyncReleaser_CreatesDelegateThatDisposesDisposableTypes() + { + // Arrange + var context = new PageContext(); + var viewContext = new ViewContext(); + var activator = new DefaultPageActivatorProvider(); + var page = new DisposablePage(); + + // Act & Assert + var disposer = activator.CreateAsyncReleaser(new CompiledPageActionDescriptor + { + PageTypeInfo = page.GetType().GetTypeInfo() + }); + Assert.NotNull(disposer); + disposer(context, viewContext, page); + + // Assert + Assert.True(page.Disposed); + } + + [Fact] + public async Task CreateAsyncReleaser_CreatesDelegateThatDisposesAsyncDisposableTypes() + { + // Arrange + var context = new PageContext(); + var viewContext = new ViewContext(); + var activator = new DefaultPageActivatorProvider(); + var page = new AsyncDisposablePage(); + + // Act & Assert + var disposer = activator.CreateAsyncReleaser(new CompiledPageActionDescriptor + { + PageTypeInfo = page.GetType().GetTypeInfo() + }); + Assert.NotNull(disposer); + await disposer(context, viewContext, page); + + // Assert + Assert.True(page.Disposed); + } + + [Fact] + public async Task CreateAsyncReleaser_CreatesDelegateThatPrefersAsyncDisposeAsyncOverDispose() + { + // Arrange + var context = new PageContext(); + var viewContext = new ViewContext(); + var activator = new DefaultPageActivatorProvider(); + var page = new DisposableAndAsyncDisposablePage(); + + // Act & Assert + var disposer = activator.CreateAsyncReleaser(new CompiledPageActionDescriptor + { + PageTypeInfo = page.GetType().GetTypeInfo() + }); + Assert.NotNull(disposer); + await disposer(context, viewContext, page); + + // Assert + Assert.True(page.AsyncDisposed); + Assert.False(page.SyncDisposed); + } + private class TestPage : Page { public override Task ExecuteAsync() @@ -155,5 +239,33 @@ public void Dispose() Disposed = true; } } + + private class AsyncDisposablePage : TestPage, IAsyncDisposable + { + public bool Disposed { get; private set; } + + public ValueTask DisposeAsync() + { + Disposed = true; + return default; + } + } + + private class DisposableAndAsyncDisposablePage : TestPage, IDisposable, IAsyncDisposable + { + public bool AsyncDisposed { get; private set; } + public bool SyncDisposed { get; private set; } + + public void Dispose() + { + SyncDisposed = true; + } + + public ValueTask DisposeAsync() + { + AsyncDisposed = true; + return default; + } + } } } diff --git a/src/Mvc/Mvc.RazorPages/test/Infrastructure/DefaultPageFactoryProviderTest.cs b/src/Mvc/Mvc.RazorPages/test/Infrastructure/DefaultPageFactoryProviderTest.cs index 9e2871d62aca..2825eaa745c5 100644 --- a/src/Mvc/Mvc.RazorPages/test/Infrastructure/DefaultPageFactoryProviderTest.cs +++ b/src/Mvc/Mvc.RazorPages/test/Infrastructure/DefaultPageFactoryProviderTest.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -289,6 +289,82 @@ public void PageFactoryDoesNotBindPropertiesWithNoRazorInjectAttribute() Assert.NotNull(testPage.ModelExpressionProviderWithInject); } + [Fact] + public void PageFactoryCreatePageDisposerCreatesDisposerForPage() + { + // Arrange + var serviceProvider = new ServiceCollection() + .AddSingleton(NullLogger.Instance) + .BuildServiceProvider(); + + var pageContext = new PageContext + { + ActionDescriptor = new CompiledPageActionDescriptor + { + PageTypeInfo = typeof(DisposablePage).GetTypeInfo() + }, + HttpContext = new DefaultHttpContext + { + RequestServices = serviceProvider, + }, + }; + + var viewContext = new ViewContext() + { + HttpContext = pageContext.HttpContext, + }; + + var factoryProvider = CreatePageFactory(); + + // Act + var factory = factoryProvider.CreatePageFactory(pageContext.ActionDescriptor); + var instance = factory(pageContext, viewContext); + + var disposer = factoryProvider.CreatePageDisposer(pageContext.ActionDescriptor); + disposer(pageContext, viewContext, instance); + + // Assert + Assert.True(((DisposablePage)instance).Disposed); + } + + [Fact] + public async Task PageFactoryCreateAsyncPageDisposerCreatesDisposerForPage() + { + // Arrange + var serviceProvider = new ServiceCollection() + .AddSingleton(NullLogger.Instance) + .BuildServiceProvider(); + + var pageContext = new PageContext + { + ActionDescriptor = new CompiledPageActionDescriptor + { + PageTypeInfo = typeof(DisposablePage).GetTypeInfo() + }, + HttpContext = new DefaultHttpContext + { + RequestServices = serviceProvider, + }, + }; + + var viewContext = new ViewContext() + { + HttpContext = pageContext.HttpContext, + }; + + var factoryProvider = CreatePageFactory(); + + // Act + var factory = factoryProvider.CreatePageFactory(pageContext.ActionDescriptor); + var instance = factory(pageContext, viewContext); + + var disposer = factoryProvider.CreateAsyncPageDisposer(pageContext.ActionDescriptor); + await disposer(pageContext, viewContext, instance); + + // Assert + Assert.True(((DisposablePage)instance).Disposed); + } + private static DefaultPageFactoryProvider CreatePageFactory( IPageActivatorProvider pageActivator = null, IModelMetadataProvider provider = null, @@ -317,6 +393,26 @@ private static IPageActivatorProvider CreateActivator() { return (context, viewContext) => Activator.CreateInstance(descriptor.PageTypeInfo.AsType()); }); + activator + .Setup(a => a.CreateReleaser(It.IsAny())) + .Returns((CompiledPageActionDescriptor descriptor) => + { + return (context, viewContext, instance) => (instance as IDisposable)?.Dispose(); + }); + + activator + .Setup(a => a.CreateAsyncReleaser(It.IsAny())) + .Returns((CompiledPageActionDescriptor descriptor) => + { + return (context, viewContext, instance) => instance switch { + IAsyncDisposable asyncDisposable => asyncDisposable.DisposeAsync(), + IDisposable disposable => SyncDispose(disposable), + _ => default + }; + }); + + ValueTask SyncDispose(IDisposable disposable) { disposable.Dispose(); return default; } + return activator.Object; } @@ -367,6 +463,20 @@ private class DerivedViewDataTestPageModel : ViewDataTestPageModel { } + private class DisposablePage : Page, IDisposable + { + public bool Disposed { get; set; } + + public void Dispose() + { + Disposed = true; + } + + public override Task ExecuteAsync() + { + throw new NotImplementedException(); + } + } private class PropertiesWithoutRazorInject : Page { diff --git a/src/Mvc/Mvc.RazorPages/test/Infrastructure/DefaultPageModelActivatorProviderTest.cs b/src/Mvc/Mvc.RazorPages/test/Infrastructure/DefaultPageModelActivatorProviderTest.cs index e2e89e5214d9..2a01032f3de9 100644 --- a/src/Mvc/Mvc.RazorPages/test/Infrastructure/DefaultPageModelActivatorProviderTest.cs +++ b/src/Mvc/Mvc.RazorPages/test/Infrastructure/DefaultPageModelActivatorProviderTest.cs @@ -1,8 +1,9 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; using System.Reflection; +using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Rendering; using Microsoft.AspNetCore.Mvc.ViewFeatures; @@ -109,6 +110,26 @@ public void CreateReleaser_ReturnsNullForModelsThatDoNotImplementDisposable(Type Assert.Null(releaser); } + [Theory] + [InlineData(typeof(SimpleModel))] + [InlineData(typeof(object))] + public void CreateAsyncReleaser_ReturnsNullForModelsThatDoNotImplementDisposable(Type pageType) + { + // Arrange + var context = new PageContext(); + var activator = new DefaultPageModelActivatorProvider(); + var actionDescriptor = new CompiledPageActionDescriptor + { + PageTypeInfo = pageType.GetTypeInfo(), + }; + + // Act + var releaser = activator.CreateAsyncReleaser(actionDescriptor); + + // Assert + Assert.Null(releaser); + } + [Fact] public void CreateReleaser_CreatesDelegateThatDisposesDisposableTypes() { @@ -131,6 +152,70 @@ public void CreateReleaser_CreatesDelegateThatDisposesDisposableTypes() Assert.True(model.Disposed); } + [Fact] + public void CreateAsyncReleaser_CreatesDelegateThatDisposesDisposableTypes() + { + // Arrange + var context = new PageContext(); + var viewContext = new ViewContext(); + var activator = new DefaultPageModelActivatorProvider(); + var model = new DisposableModel(); + + // Act & Assert + var disposer = activator.CreateAsyncReleaser(new CompiledPageActionDescriptor + { + ModelTypeInfo = model.GetType().GetTypeInfo() + }); + Assert.NotNull(disposer); + disposer(context, model); + + // Assert + Assert.True(model.Disposed); + } + + [Fact] + public async Task CreateAsyncReleaser_CreatesDelegateThatDisposesAsyncDisposableTypes() + { + // Arrange + var context = new PageContext(); + var viewContext = new ViewContext(); + var activator = new DefaultPageModelActivatorProvider(); + var model = new AsyncDisposableModel(); + + // Act & Assert + var disposer = activator.CreateAsyncReleaser(new CompiledPageActionDescriptor + { + ModelTypeInfo = model.GetType().GetTypeInfo() + }); + Assert.NotNull(disposer); + await disposer(context, model); + + // Assert + Assert.True(model.Disposed); + } + + [Fact] + public async Task CreateAsyncReleaser_CreatesDelegateThatPrefersAsyncDisposeAsyncOverDispose() + { + // Arrange + var context = new PageContext(); + var viewContext = new ViewContext(); + var activator = new DefaultPageModelActivatorProvider(); + var model = new DisposableAndAsyncDisposableModel(); + + // Act & Assert + var disposer = activator.CreateAsyncReleaser(new CompiledPageActionDescriptor + { + ModelTypeInfo = model.GetType().GetTypeInfo() + }); + Assert.NotNull(disposer); + await disposer(context, model); + + // Assert + Assert.True(model.AsyncDisposed); + Assert.False(model.SyncDisposed); + } + private class SimpleModel { } @@ -154,5 +239,33 @@ public void Dispose() Disposed = true; } } + + private class AsyncDisposableModel : IAsyncDisposable + { + public bool Disposed { get; private set; } + + public ValueTask DisposeAsync() + { + Disposed = true; + return default; + } + } + + private class DisposableAndAsyncDisposableModel : IDisposable, IAsyncDisposable + { + public bool AsyncDisposed { get; private set; } + public bool SyncDisposed { get; private set; } + + public void Dispose() + { + SyncDisposed = true; + } + + public ValueTask DisposeAsync() + { + AsyncDisposed = true; + return default; + } + } } } diff --git a/src/Mvc/Mvc.RazorPages/test/Infrastructure/DefaultPageModelFactoryProviderTest.cs b/src/Mvc/Mvc.RazorPages/test/Infrastructure/DefaultPageModelFactoryProviderTest.cs index b405f17e2a5e..5408ae372d03 100644 --- a/src/Mvc/Mvc.RazorPages/test/Infrastructure/DefaultPageModelFactoryProviderTest.cs +++ b/src/Mvc/Mvc.RazorPages/test/Infrastructure/DefaultPageModelFactoryProviderTest.cs @@ -1,8 +1,9 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; using System.Reflection; +using System.Threading.Tasks; using Moq; using Xunit; @@ -103,6 +104,28 @@ public void CreateModelDisposer_ReturnsDisposerFromModelActivatorProvider() Assert.Same(disposer, actual); } + [Fact] + public void CreateAsyncModelDisposer_ReturnsDisposerFromModelActivatorProvider() + { + // Arrange + var descriptor = new CompiledPageActionDescriptor + { + ModelTypeInfo = typeof(SimpleModel).GetTypeInfo() + }; + var pageContext = new PageContext(); + var modelActivatorProvider = new Mock(); + Func disposer = (_, __) => default; + modelActivatorProvider.Setup(p => p.CreateAsyncReleaser(descriptor)) + .Returns(disposer); + var factoryProvider = CreateModelFactoryProvider(modelActivatorProvider.Object); + + // Act + var actual = factoryProvider.CreateAsyncModelDisposer(descriptor); + + // Assert + Assert.Same(disposer, actual); + } + private static DefaultPageModelFactoryProvider CreateModelFactoryProvider( IPageModelActivatorProvider modelActivator = null) { diff --git a/src/Mvc/Mvc.RazorPages/test/Infrastructure/PageActionInvokerProviderTest.cs b/src/Mvc/Mvc.RazorPages/test/Infrastructure/PageActionInvokerProviderTest.cs index 64257793ca83..ff0fbb7248d4 100644 --- a/src/Mvc/Mvc.RazorPages/test/Infrastructure/PageActionInvokerProviderTest.cs +++ b/src/Mvc/Mvc.RazorPages/test/Infrastructure/PageActionInvokerProviderTest.cs @@ -5,6 +5,7 @@ using System.Diagnostics; using System.Linq; using System.Reflection; +using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Filters; @@ -37,7 +38,7 @@ public void OnProvidersExecuting_WithEmptyModel_PopulatesCacheEntry() }); Func factory = (a, b) => null; - Action releaser = (a, b, c) => { }; + Func releaser = (a, b, c) => default; var loader = Mock.Of(); @@ -46,7 +47,7 @@ public void OnProvidersExecuting_WithEmptyModel_PopulatesCacheEntry() .Setup(f => f.CreatePageFactory(It.IsAny())) .Returns(factory); pageFactoryProvider - .Setup(f => f.CreatePageDisposer(It.IsAny())) + .Setup(f => f.CreateAsyncPageDisposer(It.IsAny())) .Returns(releaser); var invokerProvider = CreateInvokerProvider( @@ -89,9 +90,9 @@ public void OnProvidersExecuting_WithModel_PopulatesCacheEntry() modelType: typeof(DerivedTestPageModel)); Func factory = (a, b) => null; - Action releaser = (a, b, c) => { }; + Func releaser = (a, b, c) => default; Func modelFactory = _ => null; - Action modelDisposer = (_, __) => { }; + Func modelDisposer = (_, __) => default; var loader = Mock.Of(); var pageFactoryProvider = new Mock(); @@ -99,7 +100,7 @@ public void OnProvidersExecuting_WithModel_PopulatesCacheEntry() .Setup(f => f.CreatePageFactory(It.IsAny())) .Returns(factory); pageFactoryProvider - .Setup(f => f.CreatePageDisposer(It.IsAny())) + .Setup(f => f.CreateAsyncPageDisposer(It.IsAny())) .Returns(releaser); var modelFactoryProvider = new Mock(); @@ -107,7 +108,7 @@ public void OnProvidersExecuting_WithModel_PopulatesCacheEntry() .Setup(f => f.CreateModelFactory(It.IsAny())) .Returns(modelFactory); modelFactoryProvider - .Setup(f => f.CreateModelDisposer(It.IsAny())) + .Setup(f => f.CreateAsyncModelDisposer(It.IsAny())) .Returns(modelDisposer); var invokerProvider = CreateInvokerProvider( diff --git a/src/Mvc/Mvc.RazorPages/test/Infrastructure/PageActionInvokerTest.cs b/src/Mvc/Mvc.RazorPages/test/Infrastructure/PageActionInvokerTest.cs index de4af69d0c22..5eca5714d96c 100644 --- a/src/Mvc/Mvc.RazorPages/test/Infrastructure/PageActionInvokerTest.cs +++ b/src/Mvc/Mvc.RazorPages/test/Infrastructure/PageActionInvokerTest.cs @@ -1552,9 +1552,9 @@ object pageFactory(PageContext context, ViewContext viewContext) actionDescriptor, viewDataFactory, pageFactory, - (c, viewContext, page) => { (page as IDisposable)?.Dispose(); }, + (c, viewContext, page) => { (page as IDisposable)?.Dispose(); return default; }, modelFactory, - (c, model) => { (model as IDisposable)?.Dispose(); }, + (c, model) => { (model as IDisposable)?.Dispose(); return default; }, null, handlers, handlerBinders, diff --git a/src/Mvc/Mvc.ViewFeatures/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.ViewFeatures/src/PublicAPI.Unshipped.txt index 7dc5c58110bf..d3719afc2d32 100644 --- a/src/Mvc/Mvc.ViewFeatures/src/PublicAPI.Unshipped.txt +++ b/src/Mvc/Mvc.ViewFeatures/src/PublicAPI.Unshipped.txt @@ -1 +1,4 @@ #nullable enable +~Microsoft.AspNetCore.Mvc.ViewComponents.DefaultViewComponentFactory.ReleaseViewComponentAsync(Microsoft.AspNetCore.Mvc.ViewComponents.ViewComponentContext context, object component) -> System.Threading.Tasks.ValueTask +~Microsoft.AspNetCore.Mvc.ViewComponents.IViewComponentActivator.ReleaseAsync(Microsoft.AspNetCore.Mvc.ViewComponents.ViewComponentContext context, object viewComponent) -> System.Threading.Tasks.ValueTask +~Microsoft.AspNetCore.Mvc.ViewComponents.IViewComponentFactory.ReleaseViewComponentAsync(Microsoft.AspNetCore.Mvc.ViewComponents.ViewComponentContext context, object component) -> System.Threading.Tasks.ValueTask diff --git a/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/DefaultViewComponentActivator.cs b/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/DefaultViewComponentActivator.cs index f4b15090169d..3075384c9356 100644 --- a/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/DefaultViewComponentActivator.cs +++ b/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/DefaultViewComponentActivator.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.ViewFeatures; @@ -77,5 +78,26 @@ public void Release(ViewComponentContext context, object viewComponent) disposable.Dispose(); } } + + public ValueTask ReleaseAsync(ViewComponentContext context, object viewComponent) + { + if (context == null) + { + throw new InvalidOperationException(nameof(context)); + } + + if (viewComponent == null) + { + throw new InvalidOperationException(nameof(viewComponent)); + } + + if (viewComponent is IAsyncDisposable disposable) + { + return disposable.DisposeAsync(); + } + + Release(context, viewComponent); + return default; + } } } diff --git a/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/DefaultViewComponentFactory.cs b/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/DefaultViewComponentFactory.cs index f0f77ac59dfc..d75a463a023b 100644 --- a/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/DefaultViewComponentFactory.cs +++ b/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/DefaultViewComponentFactory.cs @@ -1,9 +1,10 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; using System.Collections.Concurrent; using System.Reflection; +using System.Threading.Tasks; using Microsoft.Extensions.Internal; namespace Microsoft.AspNetCore.Mvc.ViewComponents @@ -88,5 +89,21 @@ public void ReleaseViewComponent(ViewComponentContext context, object component) _activator.Release(context, component); } + + /// + public ValueTask ReleaseViewComponentAsync(ViewComponentContext context, object component) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (component == null) + { + throw new ArgumentNullException(nameof(component)); + } + + return _activator.ReleaseAsync(context, component); + } } } diff --git a/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/DefaultViewComponentInvoker.cs b/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/DefaultViewComponentInvoker.cs index 64fafcd6f2d7..f6ff1ae5b718 100644 --- a/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/DefaultViewComponentInvoker.cs +++ b/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/DefaultViewComponentInvoker.cs @@ -81,24 +81,34 @@ public async Task InvokeAsync(ViewComponentContext context) } IViewComponentResult result; - if (executor.IsMethodAsync) + object? component = null; + try { - result = await InvokeAsyncCore(executor, context); + component = _viewComponentFactory.CreateViewComponent(context); + if (executor.IsMethodAsync) + { + result = await InvokeAsyncCore(executor, component, context); + } + else + { + // We support falling back to synchronous if there is no InvokeAsync method, in this case we'll still + // execute the IViewResult asynchronously. + result = InvokeSyncCore(executor, component, context); + } } - else + finally { - // We support falling back to synchronous if there is no InvokeAsync method, in this case we'll still - // execute the IViewResult asynchronously. - result = InvokeSyncCore(executor, context); + if (component != null) + { + await _viewComponentFactory.ReleaseViewComponentAsync(context, executor); + } } await result.ExecuteAsync(context); } - private async Task InvokeAsyncCore(ObjectMethodExecutor executor, ViewComponentContext context) + private async Task InvokeAsyncCore(ObjectMethodExecutor executor, object component, ViewComponentContext context) { - var component = _viewComponentFactory.CreateViewComponent(context); - using (_logger.ViewComponentScope(context)) { var arguments = PrepareArguments(context.Arguments, executor); @@ -150,16 +160,12 @@ private async Task InvokeAsyncCore(ObjectMethodExecutor ex _logger.ViewComponentExecuted(context, stopwatch.GetElapsedTime(), viewComponentResult); _diagnosticListener.AfterViewComponent(context, viewComponentResult, component); - _viewComponentFactory.ReleaseViewComponent(context, component); - return viewComponentResult; } } - private IViewComponentResult InvokeSyncCore(ObjectMethodExecutor executor, ViewComponentContext context) + private IViewComponentResult InvokeSyncCore(ObjectMethodExecutor executor, object component, ViewComponentContext context) { - var component = _viewComponentFactory.CreateViewComponent(context); - using (_logger.ViewComponentScope(context)) { var arguments = PrepareArguments(context.Arguments, executor); @@ -170,21 +176,12 @@ private IViewComponentResult InvokeSyncCore(ObjectMethodExecutor executor, ViewC var stopwatch = ValueStopwatch.StartNew(); object? result; - try - { result = executor.Execute(component, arguments); - } - finally - { - _viewComponentFactory.ReleaseViewComponent(context, component); - } var viewComponentResult = CoerceToViewComponentResult(result); _logger.ViewComponentExecuted(context, stopwatch.GetElapsedTime(), viewComponentResult); _diagnosticListener.AfterViewComponent(context, viewComponentResult, component); - _viewComponentFactory.ReleaseViewComponent(context, component); - return viewComponentResult; } } diff --git a/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/IViewComponentActivator.cs b/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/IViewComponentActivator.cs index 92c3b4971b34..fe3c3360b822 100644 --- a/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/IViewComponentActivator.cs +++ b/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/IViewComponentActivator.cs @@ -1,6 +1,8 @@ // 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.Threading.Tasks; + namespace Microsoft.AspNetCore.Mvc.ViewComponents { /// @@ -24,5 +26,19 @@ public interface IViewComponentActivator /// /// The to release. void Release(ViewComponentContext context, object viewComponent); + + /// + /// Releases a ViewComponent instance. + /// + /// + /// The associated with the . + /// + /// The to release. + /// A that completes when the view component has been disposed. + ValueTask ReleaseAsync(ViewComponentContext context, object viewComponent) + { + Release(context, viewComponent); + return default; + } } -} \ No newline at end of file +} diff --git a/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/IViewComponentFactory.cs b/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/IViewComponentFactory.cs index e3ff201f7096..8bdefd54f8ab 100644 --- a/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/IViewComponentFactory.cs +++ b/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/IViewComponentFactory.cs @@ -1,6 +1,8 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Threading.Tasks; + namespace Microsoft.AspNetCore.Mvc.ViewComponents { /// @@ -21,5 +23,17 @@ public interface IViewComponentFactory /// The context associated with the . /// The view component. void ReleaseViewComponent(ViewComponentContext context, object component); + + /// + /// Releases a view component instance asynchronously. + /// + /// The context associated with the . + /// The view component. + /// A that completes when the view component has been released. + ValueTask ReleaseViewComponentAsync(ViewComponentContext context, object component) + { + ReleaseViewComponent(context, component); + return default; + } } } diff --git a/src/Mvc/Mvc.ViewFeatures/src/ViewEngines/CompositeViewEngine.cs b/src/Mvc/Mvc.ViewFeatures/src/ViewEngines/CompositeViewEngine.cs index d0478b2aeb9d..58ff50757711 100644 --- a/src/Mvc/Mvc.ViewFeatures/src/ViewEngines/CompositeViewEngine.cs +++ b/src/Mvc/Mvc.ViewFeatures/src/ViewEngines/CompositeViewEngine.cs @@ -53,6 +53,10 @@ public ViewEngineResult FindView(ActionContext context, string viewName, bool is var result = ViewEngines[i].FindView(context, viewName, isMainPage); if (result.Success) { + if (result.View is IAsyncDisposable) + { + throw new InvalidOperationException("Async disposable views are not supported."); + } return result; } @@ -125,4 +129,4 @@ public ViewEngineResult GetView(string executingFilePath, string viewPath, bool return ViewEngineResult.NotFound(viewPath, searchedLocations ?? Enumerable.Empty()); } } -} \ No newline at end of file +} diff --git a/src/Mvc/Mvc.ViewFeatures/test/ViewComponents/DefaultViewComponentActivatorTests.cs b/src/Mvc/Mvc.ViewFeatures/test/ViewComponents/DefaultViewComponentActivatorTests.cs index 134282bf8646..2c2e27e0d74b 100644 --- a/src/Mvc/Mvc.ViewFeatures/test/ViewComponents/DefaultViewComponentActivatorTests.cs +++ b/src/Mvc/Mvc.ViewFeatures/test/ViewComponents/DefaultViewComponentActivatorTests.cs @@ -63,6 +63,24 @@ public void DefaultViewComponentActivator_ActivatesViewComponentContext_IgnoresN Assert.Null(instance.C); } + [Fact] + public async Task DefaultViewComponentActivator_ReleaseAsync_PrefersAsyncDisposableOverDisposable() + { + // Arrange + var instance = new SyncAndAsyncDisposableViewComponent(); + + var activator = new DefaultViewComponentActivator(Mock.Of()); + + var context = CreateContext(typeof(SyncAndAsyncDisposableViewComponent)); + + // Act + await activator.ReleaseAsync(context, instance); + + // Assert + Assert.True(instance.AsyncDisposed); + Assert.False(instance.SyncDisposed); + } + private static ViewComponentContext CreateContext(Type componentType) { return new ViewComponentContext @@ -94,5 +112,65 @@ private class VisibilityViewComponent : ViewComponent [ViewComponentContext] protected internal ViewComponentContext C { get; set; } } + + + public class ActivablePropertiesViewComponent : IDisposable + { + [ViewComponentContext] + public ViewComponentContext Context { get; set; } + + public bool Disposed { get; private set; } + + public void Dispose() + { + Disposed = true; + } + + public string Invoke() + { + return "something"; + } + } + + public class AsyncDisposableViewComponent : IAsyncDisposable + { + [ViewComponentContext] + public ViewComponentContext Context { get; set; } + + public bool Disposed { get; private set; } + + public ValueTask DisposeAsync() + { + Disposed = true; + return default; + } + + public string Invoke() + { + return "something"; + } + } + + public class SyncAndAsyncDisposableViewComponent : IDisposable, IAsyncDisposable + { + [ViewComponentContext] + public ViewComponentContext Context { get; set; } + + public bool AsyncDisposed { get; private set; } + public bool SyncDisposed { get; private set; } + + public void Dispose() => SyncDisposed = true; + + public ValueTask DisposeAsync() + { + AsyncDisposed = true; + return default; + } + + public string Invoke() + { + return "something"; + } + } } } diff --git a/src/Mvc/Mvc.ViewFeatures/test/ViewComponents/DefaultViewComponentFactoryTest.cs b/src/Mvc/Mvc.ViewFeatures/test/ViewComponents/DefaultViewComponentFactoryTest.cs index 0bd612220d3f..3e959ab8b4a4 100644 --- a/src/Mvc/Mvc.ViewFeatures/test/ViewComponents/DefaultViewComponentFactoryTest.cs +++ b/src/Mvc/Mvc.ViewFeatures/test/ViewComponents/DefaultViewComponentFactoryTest.cs @@ -1,7 +1,8 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Threading.Tasks; using Moq; using Xunit; @@ -56,6 +57,54 @@ public void ReleaseViewComponent_CallsDispose_OnTheInstance() // Assert Assert.True(component.Disposed); } + + [Fact] + public async Task ReleaseViewComponentAsync_CallsDispose_OnTheInstance() + { + // Arrange + var context = new ViewComponentContext + { + }; + + var component = new ActivablePropertiesViewComponent(); + + var viewComponentActivator = new Mock(); + viewComponentActivator.Setup(vca => vca.ReleaseAsync(context, component)) + .Callback((c, o) => (o as IDisposable)?.Dispose()) + .Returns(default(ValueTask)); + + var factory = new DefaultViewComponentFactory(viewComponentActivator.Object); + + // Act + await factory.ReleaseViewComponentAsync(context, component); + + // Assert + Assert.True(component.Disposed); + } + + [Fact] + public async Task ReleaseViewComponentAsync_CallsDisposeAsync_OnAsyncDisposableComponents() + { + // Arrange + var context = new ViewComponentContext + { + }; + + var component = new AsyncDisposableViewComponent(); + + var viewComponentActivator = new Mock(); + viewComponentActivator.Setup(vca => vca.ReleaseAsync(context, component)) + .Callback((c, o) => (o as IAsyncDisposable)?.DisposeAsync()) + .Returns(default(ValueTask)); + + var factory = new DefaultViewComponentFactory(viewComponentActivator.Object); + + // Act + await factory.ReleaseViewComponentAsync(context, component); + + // Assert + Assert.True(component.Disposed); + } } public class ActivablePropertiesViewComponent : IDisposable @@ -75,4 +124,45 @@ public string Invoke() return "something"; } } + + public class AsyncDisposableViewComponent : IAsyncDisposable + { + [ViewComponentContext] + public ViewComponentContext Context { get; set; } + + public bool Disposed { get; private set; } + + public ValueTask DisposeAsync() + { + Disposed = true; + return default; + } + + public string Invoke() + { + return "something"; + } + } + + public class SyncAndAsyncDisposableViewComponent : IDisposable, IAsyncDisposable + { + [ViewComponentContext] + public ViewComponentContext Context { get; set; } + + public bool AsyncDisposed { get; private set; } + public bool SyncDisposed { get; private set; } + + public void Dispose() => SyncDisposed = true; + + public ValueTask DisposeAsync() + { + AsyncDisposed = true; + return default; + } + + public string Invoke() + { + return "something"; + } + } } diff --git a/src/Mvc/test/Mvc.FunctionalTests/AsyncDisposalTest.cs b/src/Mvc/test/Mvc.FunctionalTests/AsyncDisposalTest.cs new file mode 100644 index 000000000000..005dbc6dd75f --- /dev/null +++ b/src/Mvc/test/Mvc.FunctionalTests/AsyncDisposalTest.cs @@ -0,0 +1,68 @@ +// 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.Linq; +using System.Net; +using System.Net.Http; +using System.Threading.Tasks; +using BasicWebSite.Controllers; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.FunctionalTests +{ + public class AsyncDisposalTest : IClassFixture> + { + public AsyncDisposalTest(MvcTestFixture fixture) + { + Factory = fixture.Factories.FirstOrDefault() ?? fixture.WithWebHostBuilder(ConfigureWebHostBuilder); + Client = Factory.CreateDefaultClient(); + } + + private static void ConfigureWebHostBuilder(IWebHostBuilder builder) => + builder.UseStartup() + .ConfigureServices(s => s.AddSingleton()); + + public WebApplicationFactory Factory { get; } + + public HttpClient Client { get; } + + [Fact] + public async Task CanDisposeAsyncController() + { + // Arrange & Act + var sink = Factory.Services.GetRequiredService(); + var response = await Client.GetAsync("http://localhost/Disposal/DisposeMode/Async(false)/Throws(false)"); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.True(sink.DisposeAsyncInvoked); + } + + [Fact] + public async Task HandlesAsyncExceptionsDuringAsyncDisposal() + { + // Arrange & Act + var sink = Factory.Services.GetRequiredService(); + var response = await Client.GetAsync("http://localhost/Disposal/DisposeMode/Async(true)/Throws(true)"); + + // Assert + Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode); + Assert.True(sink.DisposeAsyncInvoked); + } + + [Fact] + public async Task HandlesSyncExceptionsDuringAsyncDisposal() + { + // Arrange & Act + var sink = Factory.Services.GetRequiredService(); + var response = await Client.GetAsync("http://localhost/Disposal/DisposeMode/Async(false)/Throws(true)"); + + // Assert + Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode); + Assert.True(sink.DisposeAsyncInvoked); + } + } +} diff --git a/src/Mvc/test/Mvc.FunctionalTests/Infrastructure/MvcTestFixture.cs b/src/Mvc/test/Mvc.FunctionalTests/Infrastructure/MvcTestFixture.cs index 1c85f8412d7b..e157bf10f1e7 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/Infrastructure/MvcTestFixture.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/Infrastructure/MvcTestFixture.cs @@ -26,6 +26,7 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) var testSink = new TestSink(); var loggerFactory = new TestLoggerFactory(testSink, enabled: true); services.AddSingleton(loggerFactory); + services.AddSingleton(testSink); }); } diff --git a/src/Mvc/test/Mvc.FunctionalTests/PageAsyncDisposalTest.cs b/src/Mvc/test/Mvc.FunctionalTests/PageAsyncDisposalTest.cs new file mode 100644 index 000000000000..3ce6e1222dd0 --- /dev/null +++ b/src/Mvc/test/Mvc.FunctionalTests/PageAsyncDisposalTest.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.Linq; +using System.Net; +using System.Net.Http; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.Extensions.DependencyInjection; +using RazorPagesWebSite; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.FunctionalTests +{ + public class PageAsyncDisposalTest : IClassFixture> + { + public PageAsyncDisposalTest(MvcTestFixture fixture) + { + Factory = fixture.Factories.FirstOrDefault() ?? fixture.WithWebHostBuilder(ConfigureWebHostBuilder); + Client = Factory.CreateDefaultClient(); + } + + private static void ConfigureWebHostBuilder(IWebHostBuilder builder) => + builder.UseStartup() + .ConfigureServices(s => s.AddSingleton()); + + public WebApplicationFactory Factory { get; } + + public HttpClient Client { get; } + + [Fact] + public async Task CanDisposeAsyncPage() + { + // Arrange & Act + var sink = Factory.Services.GetRequiredService(); + var response = await Client.GetAsync("http://localhost/AsyncDisposable"); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.True(sink.DisposeAsyncInvoked); + } + } +} diff --git a/src/Mvc/test/WebSites/BasicWebSite/Controllers/AsyncDisposableController.cs b/src/Mvc/test/WebSites/BasicWebSite/Controllers/AsyncDisposableController.cs new file mode 100644 index 000000000000..338ba2b178db --- /dev/null +++ b/src/Mvc/test/WebSites/BasicWebSite/Controllers/AsyncDisposableController.cs @@ -0,0 +1,51 @@ +using System; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Logging; + +namespace BasicWebSite.Controllers +{ + public class AsyncDisposableController : Controller, IAsyncDisposable + { + private readonly ControllerTestDisposeAsync _testDisposeAsync; + + public AsyncDisposableController(ILogger logger, ControllerTestDisposeAsync testDisposeAsync) + { + Logger = logger; + _testDisposeAsync = testDisposeAsync; + } + + public ILogger Logger { get; } + + public bool Async { get; private set; } + public bool Throw { get; private set; } + + [HttpGet("Disposal/DisposeMode/Async({asyncMode})/Throws({throwException})")] + public IActionResult SetDisposeMode(bool asyncMode, bool throwException) + { + Async = asyncMode; + Throw = throwException; + + return Ok(); + } + + public async ValueTask DisposeAsync() + { + _testDisposeAsync.DisposeAsyncInvoked = true; + if (Async) + { + await Task.Yield(); + } + + if (Throw) + { + throw new InvalidOperationException("Exception during disposal!"); + } + } + } + + public class ControllerTestDisposeAsync + { + public bool DisposeAsyncInvoked { get; set; } + } +} diff --git a/src/Mvc/test/WebSites/RazorPagesWebSite/AsyncDisposable.cshtml b/src/Mvc/test/WebSites/RazorPagesWebSite/AsyncDisposable.cshtml new file mode 100644 index 000000000000..7d698d2902f9 --- /dev/null +++ b/src/Mvc/test/WebSites/RazorPagesWebSite/AsyncDisposable.cshtml @@ -0,0 +1,13 @@ +@page +@implements IAsyncDisposable +@using RazorPagesWebSite +@inject PageTestDisposeAsync TestDisposeAsync +Hello, World! + +@functions{ + public ValueTask DisposeAsync() + { + TestDisposeAsync.DisposeAsyncInvoked = true; + return default; + } +} diff --git a/src/Mvc/test/WebSites/RazorPagesWebSite/PageTestDisposeAsync.cs b/src/Mvc/test/WebSites/RazorPagesWebSite/PageTestDisposeAsync.cs new file mode 100644 index 000000000000..47e68cfad8c9 --- /dev/null +++ b/src/Mvc/test/WebSites/RazorPagesWebSite/PageTestDisposeAsync.cs @@ -0,0 +1,12 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; + +namespace RazorPagesWebSite +{ + public class PageTestDisposeAsync + { + public bool DisposeAsyncInvoked { get; set; } + } +} From 57c353aa88ec1d24c699a5519ecc4721d7bb1b8d Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Tue, 19 Jan 2021 10:54:59 -0800 Subject: [PATCH 2/2] Addressed feedback --- .../src/Infrastructure/ResourceInvoker.cs | 25 +++++++++---------- src/Mvc/Mvc.Core/src/Resources.resx | 2 +- src/Mvc/Mvc.Razor/src/RazorViewEngine.cs | 2 +- src/Mvc/Mvc.Razor/src/Resources.resx | 3 +++ .../src/IPageModelActivatorProvider.cs | 2 +- .../DefaultPageFactoryProviderTest.cs | 11 +++++--- .../Infrastructure/PageActionInvokerTest.cs | 4 +-- src/Mvc/Mvc.ViewFeatures/src/Resources.resx | 3 +++ .../DefaultViewComponentInvoker.cs | 2 +- .../src/ViewEngines/CompositeViewEngine.cs | 2 +- .../PageAsyncDisposalTest.cs | 2 +- 11 files changed, 34 insertions(+), 24 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/Infrastructure/ResourceInvoker.cs b/src/Mvc/Mvc.Core/src/Infrastructure/ResourceInvoker.cs index 02a14bfb60b0..6a2fd8e2f95f 100644 --- a/src/Mvc/Mvc.Core/src/Infrastructure/ResourceInvoker.cs +++ b/src/Mvc/Mvc.Core/src/Infrastructure/ResourceInvoker.cs @@ -83,7 +83,7 @@ public virtual Task InvokeAsync() return Awaited(this, task, scope); } - return ReleaseResourcesCore(scope); + return ReleaseResourcesCore(scope).AsTask(); static async Task Awaited(ResourceInvoker invoker, Task task, IDisposable? scope) { @@ -143,27 +143,26 @@ static async Task Logged(ResourceInvoker invoker) } } - internal Task ReleaseResourcesCore(IDisposable? scope) + internal ValueTask ReleaseResourcesCore(IDisposable? scope) { Exception? releaseException = null; ValueTask releaseResult = default; try { releaseResult = ReleaseResources(); + if (!releaseResult.IsCompletedSuccessfully) + { + return HandleAsyncReleaseErrors(releaseResult, scope); + } } catch (Exception exception) { releaseException = exception; } - if (releaseException == null && !releaseResult.IsCompletedSuccessfully) - { - return HandleAsyncReleaseErrors(releaseResult, scope); - } - return HandleReleaseErrors(scope, releaseException); - static async Task HandleAsyncReleaseErrors(ValueTask releaseResult, IDisposable? scope) + static async ValueTask HandleAsyncReleaseErrors(ValueTask releaseResult, IDisposable? scope) { Exception? releaseException = null; try @@ -178,7 +177,7 @@ static async Task HandleAsyncReleaseErrors(ValueTask releaseResult, IDisposable? await HandleReleaseErrors(scope, releaseException); } - static Task HandleReleaseErrors(IDisposable? scope, Exception? releaseException) + static ValueTask HandleReleaseErrors(IDisposable? scope, Exception? releaseException) { Exception? scopeException = null; try @@ -192,19 +191,19 @@ static Task HandleReleaseErrors(IDisposable? scope, Exception? releaseException) if (releaseException == null && scopeException == null) { - return Task.CompletedTask; + return default; } else if (releaseException != null && scopeException != null) { - return Task.FromException(new AggregateException(releaseException, scopeException)); + return ValueTask.FromException(new AggregateException(releaseException, scopeException)); } else if (releaseException != null) { - return Task.FromException(releaseException); + return ValueTask.FromException(releaseException); } else { - return Task.FromException(scopeException!); + return ValueTask.FromException(scopeException!); } } } diff --git a/src/Mvc/Mvc.Core/src/Resources.resx b/src/Mvc/Mvc.Core/src/Resources.resx index acdcf3cb995c..a1e872c5be9b 100644 --- a/src/Mvc/Mvc.Core/src/Resources.resx +++ b/src/Mvc/Mvc.Core/src/Resources.resx @@ -537,4 +537,4 @@ {0} cannot update a record type model. If a '{1}' must be updated, include it in an object type. - + \ No newline at end of file diff --git a/src/Mvc/Mvc.Razor/src/RazorViewEngine.cs b/src/Mvc/Mvc.Razor/src/RazorViewEngine.cs index 1df8f759ad30..34f3e8c0dddd 100644 --- a/src/Mvc/Mvc.Razor/src/RazorViewEngine.cs +++ b/src/Mvc/Mvc.Razor/src/RazorViewEngine.cs @@ -486,7 +486,7 @@ private ViewEngineResult CreateViewEngineResult(ViewLocationCacheResult result, var view = new RazorView(this, _pageActivator, viewStarts, page, _htmlEncoder, _diagnosticListener); if (view is IAsyncDisposable) { - throw new InvalidOperationException("Async disposable views are not supported."); + throw new InvalidOperationException(Resources.FormatAsyncDisposableViewsNotSupported(typeof(IAsyncDisposable).FullName)); } return ViewEngineResult.Found(viewName, view); diff --git a/src/Mvc/Mvc.Razor/src/Resources.resx b/src/Mvc/Mvc.Razor/src/Resources.resx index bab3fddf839f..52c28820608f 100644 --- a/src/Mvc/Mvc.Razor/src/Resources.resx +++ b/src/Mvc/Mvc.Razor/src/Resources.resx @@ -200,4 +200,7 @@ At least one of the '{0}' or '{1}' values must be non-null. + + Views implementing '{0}' are not supported. + \ No newline at end of file diff --git a/src/Mvc/Mvc.RazorPages/src/IPageModelActivatorProvider.cs b/src/Mvc/Mvc.RazorPages/src/IPageModelActivatorProvider.cs index ee6107c0ecb7..27373ad5acff 100644 --- a/src/Mvc/Mvc.RazorPages/src/IPageModelActivatorProvider.cs +++ b/src/Mvc/Mvc.RazorPages/src/IPageModelActivatorProvider.cs @@ -30,7 +30,7 @@ public interface IPageModelActivatorProvider /// /// The . /// The delegate used to dispose the activated Razor Page model asynchronously. - Func CreateAsyncReleaser(CompiledPageActionDescriptor descriptor) + Func CreateAsyncReleaser(CompiledPageActionDescriptor descriptor) { var releaser = CreateReleaser(descriptor); return (context, model) => diff --git a/src/Mvc/Mvc.RazorPages/test/Infrastructure/DefaultPageFactoryProviderTest.cs b/src/Mvc/Mvc.RazorPages/test/Infrastructure/DefaultPageFactoryProviderTest.cs index 2825eaa745c5..c52688b48964 100644 --- a/src/Mvc/Mvc.RazorPages/test/Infrastructure/DefaultPageFactoryProviderTest.cs +++ b/src/Mvc/Mvc.RazorPages/test/Infrastructure/DefaultPageFactoryProviderTest.cs @@ -119,7 +119,7 @@ public void PageFactorySetsPath() { ActionDescriptor = descriptor }; - + var viewContext = new ViewContext(); // Act @@ -404,14 +404,19 @@ private static IPageActivatorProvider CreateActivator() .Setup(a => a.CreateAsyncReleaser(It.IsAny())) .Returns((CompiledPageActionDescriptor descriptor) => { - return (context, viewContext, instance) => instance switch { + return (context, viewContext, instance) => instance switch + { IAsyncDisposable asyncDisposable => asyncDisposable.DisposeAsync(), IDisposable disposable => SyncDispose(disposable), _ => default }; }); - ValueTask SyncDispose(IDisposable disposable) { disposable.Dispose(); return default; } + ValueTask SyncDispose(IDisposable disposable) + { + disposable.Dispose(); + return default; + } return activator.Object; } diff --git a/src/Mvc/Mvc.RazorPages/test/Infrastructure/PageActionInvokerTest.cs b/src/Mvc/Mvc.RazorPages/test/Infrastructure/PageActionInvokerTest.cs index 5eca5714d96c..d309d8418ba7 100644 --- a/src/Mvc/Mvc.RazorPages/test/Infrastructure/PageActionInvokerTest.cs +++ b/src/Mvc/Mvc.RazorPages/test/Infrastructure/PageActionInvokerTest.cs @@ -1400,8 +1400,8 @@ public async Task InvokeAction_ForPage_Logs(bool hasPageModel) var loggerFactory = new TestLoggerFactory(testSink, enabled: true); var logger = loggerFactory.CreateLogger("test"); - var actionDescriptor = hasPageModel - ? CreateDescriptorForPageModelPage() + var actionDescriptor = hasPageModel + ? CreateDescriptorForPageModelPage() : CreateDescriptorForSimplePage(); actionDescriptor.ViewEnginePath = "/Pages/Foo"; actionDescriptor.RouteValues.Add("page", "foo"); diff --git a/src/Mvc/Mvc.ViewFeatures/src/Resources.resx b/src/Mvc/Mvc.ViewFeatures/src/Resources.resx index ae6a29aa8173..d0f2f305d312 100644 --- a/src/Mvc/Mvc.ViewFeatures/src/Resources.resx +++ b/src/Mvc/Mvc.ViewFeatures/src/Resources.resx @@ -298,4 +298,7 @@ Unsupported RenderMode '{0}'. + + Views implementing '{0}' are not supported. + \ No newline at end of file diff --git a/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/DefaultViewComponentInvoker.cs b/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/DefaultViewComponentInvoker.cs index f6ff1ae5b718..a2f3e2ae98b7 100644 --- a/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/DefaultViewComponentInvoker.cs +++ b/src/Mvc/Mvc.ViewFeatures/src/ViewComponents/DefaultViewComponentInvoker.cs @@ -176,7 +176,7 @@ private IViewComponentResult InvokeSyncCore(ObjectMethodExecutor executor, objec var stopwatch = ValueStopwatch.StartNew(); object? result; - result = executor.Execute(component, arguments); + result = executor.Execute(component, arguments); var viewComponentResult = CoerceToViewComponentResult(result); _logger.ViewComponentExecuted(context, stopwatch.GetElapsedTime(), viewComponentResult); diff --git a/src/Mvc/Mvc.ViewFeatures/src/ViewEngines/CompositeViewEngine.cs b/src/Mvc/Mvc.ViewFeatures/src/ViewEngines/CompositeViewEngine.cs index 58ff50757711..5771f6a37d5c 100644 --- a/src/Mvc/Mvc.ViewFeatures/src/ViewEngines/CompositeViewEngine.cs +++ b/src/Mvc/Mvc.ViewFeatures/src/ViewEngines/CompositeViewEngine.cs @@ -55,7 +55,7 @@ public ViewEngineResult FindView(ActionContext context, string viewName, bool is { if (result.View is IAsyncDisposable) { - throw new InvalidOperationException("Async disposable views are not supported."); + throw new InvalidOperationException(Resources.FormatAsyncDisposableViewsNotSupported(typeof(IAsyncDisposable).FullName)); } return result; } diff --git a/src/Mvc/test/Mvc.FunctionalTests/PageAsyncDisposalTest.cs b/src/Mvc/test/Mvc.FunctionalTests/PageAsyncDisposalTest.cs index 3ce6e1222dd0..ce7125379704 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/PageAsyncDisposalTest.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/PageAsyncDisposalTest.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Linq;