From 5b6eb307ae4066ed4b9ea4fe454a91b9fdb06ca4 Mon Sep 17 00:00:00 2001 From: "N. Taylor Mullen" Date: Wed, 19 Mar 2014 16:38:29 -0700 Subject: [PATCH] Re-plumb ModelState. Modified ModelState to only ever be created on the ActionContext and then plumbed/exposed it on ViewData and Controller. This will enable: ActionFilterContext's will have access to ModelState via its ActionContext member, allow HTMLHelpers to access ModelState via ViewData, and unify the locations of "source" model state. In the old world we used to copy/replace/instantiate new model state all over unnecessarily. --- samples/MvcSample.Web/Home2Controller.cs | 10 ++++- .../ActionContext.cs | 4 ++ src/Microsoft.AspNet.Mvc.Core/Controller.cs | 23 ++++++++++-- .../DefaultControllerFactory.cs | 16 ++++---- .../IControllerFactory.cs | 3 +- .../ReflectedActionInvoker.cs | 5 +-- .../View/ViewData.cs | 37 ++++++++++++++----- .../View/ViewDataOfTModel.cs | 5 +++ .../project.json | 2 + 9 files changed, 77 insertions(+), 28 deletions(-) diff --git a/samples/MvcSample.Web/Home2Controller.cs b/samples/MvcSample.Web/Home2Controller.cs index 048d49ca34..a485abf830 100644 --- a/samples/MvcSample.Web/Home2Controller.cs +++ b/samples/MvcSample.Web/Home2Controller.cs @@ -15,7 +15,15 @@ namespace MvcSample.Web.RandomNameSpace public IActionResultHelper Result { get; private set; } - public HttpContext Context { get; set; } + public HttpContext Context + { + get + { + return ActionContext.HttpContext; + } + } + + public ActionContext ActionContext { get; set; } public string Index() { diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionContext.cs b/src/Microsoft.AspNet.Mvc.Core/ActionContext.cs index 642c39464e..cce52f4e81 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionContext.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionContext.cs @@ -1,5 +1,6 @@ using System.Collections.Generic; using Microsoft.AspNet.Abstractions; +using Microsoft.AspNet.Mvc.ModelBinding; using Microsoft.AspNet.Routing; namespace Microsoft.AspNet.Mvc @@ -12,6 +13,7 @@ namespace Microsoft.AspNet.Mvc Router = router; RouteValues = routeValues; ActionDescriptor = actionDescriptor; + ModelState = new ModelStateDictionary(); } public HttpContext HttpContext { get; private set; } @@ -20,6 +22,8 @@ namespace Microsoft.AspNet.Mvc public IDictionary RouteValues { get; private set; } + public ModelStateDictionary ModelState { get; private set; } + public ActionDescriptor ActionDescriptor { get; private set; } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/Controller.cs b/src/Microsoft.AspNet.Mvc.Core/Controller.cs index 7b76daa6ed..ff36d217f6 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Controller.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Controller.cs @@ -6,15 +6,30 @@ namespace Microsoft.AspNet.Mvc { public class Controller { - public void Initialize(IActionResultHelper actionResultHelper, IModelMetadataProvider metadataProvider) + public void Initialize(IActionResultHelper actionResultHelper) { Result = actionResultHelper; - ViewData = new ViewData(metadataProvider); } - public IActionResultHelper Result { get; private set; } + public HttpContext Context + { + get + { + return ActionContext.HttpContext; + } + } - public HttpContext Context { get; set; } + public ModelStateDictionary ModelState + { + get + { + return ViewData.ModelState; + } + } + + public ActionContext ActionContext { get; set; } + + public IActionResultHelper Result { get; private set; } public IUrlHelper Url { get; set; } diff --git a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerFactory.cs b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerFactory.cs index 56b93404df..294465f192 100644 --- a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerFactory.cs +++ b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerFactory.cs @@ -19,7 +19,7 @@ namespace Microsoft.AspNet.Mvc _activator = activator; } - public object CreateController(ActionContext actionContext, ModelStateDictionary modelState) + public object CreateController(ActionContext actionContext) { var actionDescriptor = actionContext.ActionDescriptor as ReflectedActionDescriptor; if (actionDescriptor == null) @@ -32,7 +32,7 @@ namespace Microsoft.AspNet.Mvc var controller = _activator.CreateInstance(actionDescriptor.ControllerDescriptor.ControllerTypeInfo.AsType()); // TODO: How do we feed the controller with context (need DI improvements) - InitializeController(controller, actionContext, modelState); + InitializeController(controller, actionContext); return controller; } @@ -47,21 +47,21 @@ namespace Microsoft.AspNet.Mvc { } - private void InitializeController(object controller, ActionContext actionContext, ModelStateDictionary modelState) + private void InitializeController(object controller, ActionContext actionContext) { var controllerType = controller.GetType(); foreach (var prop in controllerType.GetRuntimeProperties()) { - if (prop.Name == "Context" && prop.PropertyType == typeof(HttpContext)) + if(prop.Name == "ActionContext" && prop.PropertyType.GetTypeInfo().IsAssignableFrom(typeof(ActionContext).GetTypeInfo())) { - prop.SetValue(controller, actionContext.HttpContext); + prop.SetValue(controller, actionContext); } - else if (prop.Name == "ModelState" && prop.PropertyType == typeof(ModelStateDictionary)) + else if (prop.Name == "ViewData" && prop.PropertyType.GetTypeInfo().IsAssignableFrom(typeof(ViewData).GetTypeInfo())) { - prop.SetValue(controller, modelState); + prop.SetValue(controller, new ViewData(_serviceProvider.GetService(), actionContext.ModelState)); } - else if (prop.Name == "Url" && prop.PropertyType == typeof(IUrlHelper)) + else if (prop.Name == "Url" && prop.PropertyType.GetTypeInfo().IsAssignableFrom(typeof(IUrlHelper).GetTypeInfo())) { var urlHelper = new UrlHelper( actionContext.HttpContext, diff --git a/src/Microsoft.AspNet.Mvc.Core/IControllerFactory.cs b/src/Microsoft.AspNet.Mvc.Core/IControllerFactory.cs index db4f820bcb..949fd2dd8d 100644 --- a/src/Microsoft.AspNet.Mvc.Core/IControllerFactory.cs +++ b/src/Microsoft.AspNet.Mvc.Core/IControllerFactory.cs @@ -1,11 +1,10 @@  -using Microsoft.AspNet.Mvc.ModelBinding; namespace Microsoft.AspNet.Mvc { public interface IControllerFactory { - object CreateController(ActionContext actionContext, ModelStateDictionary modelState); + object CreateController(ActionContext actionContext); void ReleaseController(object controller); } diff --git a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvoker.cs b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvoker.cs index 777b501db1..455b5c7fd7 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvoker.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvoker.cs @@ -50,8 +50,7 @@ namespace Microsoft.AspNet.Mvc PreArrangeFiltersInPipeline(filterProviderContext); - var modelState = new ModelStateDictionary(); - object controller = _controllerFactory.CreateController(_actionContext, modelState); + var controller = _controllerFactory.CreateController(_actionContext); if (controller == null) { @@ -96,7 +95,7 @@ namespace Microsoft.AspNet.Mvc if (actionResult == null) { - var parameterValues = await GetParameterValues(modelState); + var parameterValues = await GetParameterValues(_actionContext.ModelState); var actionFilterContext = new ActionFilterContext(_actionContext, filterMetaItems, diff --git a/src/Microsoft.AspNet.Mvc.Rendering/View/ViewData.cs b/src/Microsoft.AspNet.Mvc.Rendering/View/ViewData.cs index 1d08f9f6a8..a02fbe762a 100644 --- a/src/Microsoft.AspNet.Mvc.Rendering/View/ViewData.cs +++ b/src/Microsoft.AspNet.Mvc.Rendering/View/ViewData.cs @@ -13,16 +13,32 @@ namespace Microsoft.AspNet.Mvc.Rendering private IModelMetadataProvider _metadataProvider; public ViewData([NotNull] IModelMetadataProvider metadataProvider) + : this(metadataProvider, new ModelStateDictionary()) { + } + + public ViewData([NotNull] IModelMetadataProvider metadataProvider, [NotNull] ModelStateDictionary modelState) + { + ModelState = modelState; _data = new Dictionary(); _metadataProvider = metadataProvider; } public ViewData([NotNull] ViewData source) + : this(source.MetadataProvider) { - _data = source._data; _modelMetadata = source.ModelMetadata; - _metadataProvider = source.MetadataProvider; + + foreach (var entry in source.ModelState) + { + ModelState.Add(entry.Key, entry.Value); + } + + foreach (var entry in source) + { + _data.Add(entry.Key, entry.Value); + } + SetModel(source.Model); } @@ -32,19 +48,15 @@ namespace Microsoft.AspNet.Mvc.Rendering set { SetModel(value); } } + public ModelStateDictionary ModelState { get; private set; } + public dynamic this[string index] { get { dynamic result; - if (_data.TryGetValue(index, out result)) - { - result = _data[index]; - } - else - { - result = null; - } + + _data.TryGetValue(index, out result); return result; } @@ -73,6 +85,11 @@ namespace Microsoft.AspNet.Mvc.Rendering { get { return _metadataProvider; } } + + public Dictionary.Enumerator GetEnumerator() + { + return _data.GetEnumerator(); + } public override bool TryGetMember(GetMemberBinder binder, out object result) { diff --git a/src/Microsoft.AspNet.Mvc.Rendering/View/ViewDataOfTModel.cs b/src/Microsoft.AspNet.Mvc.Rendering/View/ViewDataOfTModel.cs index b84e356674..6c658da44e 100644 --- a/src/Microsoft.AspNet.Mvc.Rendering/View/ViewDataOfTModel.cs +++ b/src/Microsoft.AspNet.Mvc.Rendering/View/ViewDataOfTModel.cs @@ -14,6 +14,11 @@ namespace Microsoft.AspNet.Mvc.Rendering { _defaultModelMetadata = MetadataProvider.GetMetadataForType(null, typeof(TModel)); } + + public ViewData([NotNull] IModelMetadataProvider metadataProvider, [NotNull] ModelStateDictionary modelState) + : base(metadataProvider, modelState) + { + } public ViewData(ViewData source) : base(source) diff --git a/src/Microsoft.AspNet.Mvc.Rendering/project.json b/src/Microsoft.AspNet.Mvc.Rendering/project.json index 9cbcf5ec53..f3bd2f409b 100644 --- a/src/Microsoft.AspNet.Mvc.Rendering/project.json +++ b/src/Microsoft.AspNet.Mvc.Rendering/project.json @@ -10,6 +10,7 @@ "net45": {}, "k10": { "dependencies": { + "Microsoft.CSharp": "4.0.0.0", "System.Collections": "4.0.0.0", "System.ComponentModel": "4.0.0.0", "System.Diagnostics.Contracts": "4.0.0.0", @@ -19,6 +20,7 @@ "System.Globalization": "4.0.10.0", "System.IO": "4.0.0.0", "System.Linq": "4.0.0.0", + "System.Linq.Expressions": "4.0.0.0", "System.Reflection": "4.0.10.0", "System.Reflection.Extensions": "4.0.0.0", "System.Resources.ResourceManager": "4.0.0.0",