diff --git a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/ApplicationModelConventionExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/ApplicationModelConventionExtensions.cs index 4ed7ba369f..b23f9d7bed 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/ApplicationModelConventionExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/ApplicationModelConventionExtensions.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.Linq; using System.Collections.Generic; using Microsoft.AspNetCore.Mvc.ApplicationModels; @@ -148,11 +149,16 @@ namespace Microsoft.Extensions.DependencyInjection throw new ArgumentNullException(nameof(application)); } - foreach (var controller in application.Controllers) + // Create copies of collections of controllers, actions and parameters as users could modify + // these collections from within the convention itself. + var controllers = application.Controllers.ToArray(); + foreach (var controller in controllers) { - foreach (var action in controller.Actions) + var actions = controller.Actions.ToArray(); + foreach (var action in actions) { - foreach (var parameter in action.Parameters) + var parameters = action.Parameters.ToArray(); + foreach (var parameter in parameters) { _parameterModelConvention.Apply(parameter); } @@ -183,9 +189,13 @@ namespace Microsoft.Extensions.DependencyInjection throw new ArgumentNullException(nameof(application)); } - foreach (var controller in application.Controllers) + // Create copies of collections of controllers, actions and parameters as users could modify + // these collections from within the convention itself. + var controllers = application.Controllers.ToArray(); + foreach (var controller in controllers) { - foreach (var action in controller.Actions) + var actions = controller.Actions.ToArray(); + foreach (var action in actions) { _actionModelConvention.Apply(action); } @@ -215,7 +225,8 @@ namespace Microsoft.Extensions.DependencyInjection throw new ArgumentNullException(nameof(application)); } - foreach (var controller in application.Controllers) + var controllers = application.Controllers.ToArray(); + foreach (var controller in controllers) { _controllerModelConvention.Apply(controller); } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/ApplicationModelConventionExtensionsTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/ApplicationModelConventionExtensionsTest.cs index e635763920..134fdfb688 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/ApplicationModelConventionExtensionsTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/ApplicationModelConventionExtensionsTest.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Reflection; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.ApplicationModels; +using Microsoft.AspNetCore.Mvc.Internal; using Xunit; namespace Microsoft.Extensions.DependencyInjection @@ -108,6 +109,82 @@ namespace Microsoft.Extensions.DependencyInjection Assert.IsType(convention); } + [Fact] + public void ApplicationModelConventions_CopiesControllerModelCollectionOnApply() + { + // Arrange + var applicationModel = new ApplicationModel(); + applicationModel.Controllers.Add( + new ControllerModel(typeof(HelloController).GetTypeInfo(), new List()) + { + Application = applicationModel + }); + + var controllerModelConvention = new ControllerModelCollectionModifyingConvention(); + var conventions = new List(); + conventions.Add(controllerModelConvention); + + // Act & Assert + ApplicationModelConventions.ApplyConventions(applicationModel, conventions); + } + + [Fact] + public void ApplicationModelConventions_CopiesActionModelCollectionOnApply() + { + // Arrange + var controllerType = typeof(HelloController).GetTypeInfo(); + var applicationModel = new ApplicationModel(); + var controllerModel = new ControllerModel(controllerType, new List()) + { + Application = applicationModel + }; + controllerModel.Actions.Add( + new ActionModel(controllerType.GetMethod(nameof(HelloController.GetHello)), new List()) + { + Controller = controllerModel + }); + applicationModel.Controllers.Add(controllerModel); + + var actionModelConvention = new ActionModelCollectionModifyingConvention(); + var conventions = new List(); + conventions.Add(actionModelConvention); + + // Act & Assert + ApplicationModelConventions.ApplyConventions(applicationModel, conventions); + } + + [Fact] + public void ApplicationModelConventions_CopiesParameterModelCollectionOnApply() + { + // Arrange + var controllerType = typeof(HelloController).GetTypeInfo(); + var app = new ApplicationModel(); + var controllerModel = new ControllerModel(controllerType, new List()) + { + Application = app + }; + app.Controllers.Add(controllerModel); + var actionModel = new ActionModel(controllerType.GetMethod(nameof(HelloController.GetInfo)), new List()) + { + Controller = controllerModel + }; + controllerModel.Actions.Add(actionModel); + var parameterModel = new ParameterModel( + controllerType.GetMethod(nameof(HelloController.GetInfo)).GetParameters()[0], + new List()) + { + Action = actionModel + }; + actionModel.Parameters.Add(parameterModel); + + var parameterModelConvention = new ParameterModelCollectionModifyingConvention(); + var conventions = new List(); + conventions.Add(parameterModelConvention); + + // Act & Assert + ApplicationModelConventions.ApplyConventions(app, conventions); + } + [Fact] public void GenericRemoveType_RemovesAllOfType() { @@ -149,6 +226,11 @@ namespace Microsoft.Extensions.DependencyInjection { return "Hello"; } + + public string GetInfo(int id) + { + return "GetInfo(int id)"; + } } private class WorldController @@ -182,5 +264,37 @@ namespace Microsoft.Extensions.DependencyInjection controller.Properties.Add("TestProperty", "TestValue"); } } + + private class ControllerModelCollectionModifyingConvention : IControllerModelConvention + { + public void Apply(ControllerModel controller) + { + controller.Application.Controllers.Remove(controller); + } + } + + private class TestApplicationModelConvention : IApplicationModelConvention + { + public void Apply(ApplicationModel application) + { + application.Controllers.RemoveAt(0); + } + } + + private class ActionModelCollectionModifyingConvention : IActionModelConvention + { + public void Apply(ActionModel action) + { + action.Controller.Actions.Remove(action); + } + } + + private class ParameterModelCollectionModifyingConvention : IParameterModelConvention + { + public void Apply(ParameterModel parameter) + { + parameter.Action.Parameters.Remove(parameter); + } + } } } \ No newline at end of file