From 8ed5b7b0797844da65176f7110eea5cd796f37dc Mon Sep 17 00:00:00 2001 From: dougbu Date: Tue, 25 Mar 2014 11:57:56 -0700 Subject: [PATCH] Fix WebFx-169 and #118 - move `DynamicObject` derivation up to new `DynamicViewData` class, fixing [WebFx-169](http://projectk-tc:8080/browse/WEBFX-169) - avoid direct `_data` lookup in previous `TryGetMember()`, fixing [#118](https://github.com/aspnet/WebFx/issues/118) - rename ViewData -> ViewDataDictionary Also - flesh out `IDictionary` implementation in `ViewData` - provide `ViewData` copy constructor that allows TModel to change - remove `TryGetIndex()` and `TrySetIndex()` implementations; use `ViewData[]` instead - restore `ViewContext.ViewBag` from legacy MVC --- .../MvcSample.Web/Views/Home/Create.cshtml | 11 +- .../ActionResultHelper.cs | 2 +- .../ActionResults/ViewResult.cs | 2 +- src/Microsoft.AspNet.Mvc.Core/Controller.cs | 14 +- .../DefaultControllerFactory.cs | 12 +- .../IActionResultHelper.cs | 2 +- .../DefaultViewComponentInvoker.cs | 4 +- .../DefaultViewComponentResultHelper.cs | 2 +- .../IViewComponentResultHelper.cs | 2 +- .../ViewComponents/ViewComponent.cs | 4 +- .../ViewComponents/ViewViewComponentResult.cs | 5 +- src/Microsoft.AspNet.Mvc.Razor/RazorView.cs | 8 + .../RazorViewOfT.cs | 15 +- .../Html/HtmlHelper.cs | 4 +- .../Html/HtmlHelperOfT.cs | 4 +- .../Properties/Resources.Designer.cs | 16 ++ .../Properties/Resources.resx | 7 +- .../View/DynamicViewData.cs | 55 +++++ .../View/ViewContext.cs | 17 +- .../View/ViewData.cs | 155 -------------- .../View/ViewDataDictionary.cs | 198 ++++++++++++++++++ ...taOfTModel.cs => ViewDataDictionaryOfT.cs} | 24 ++- .../ControllerTests.cs | 31 +++ .../project.json | 2 + .../ViewContextTests.cs | 28 +++ .../ViewDataOfTTest.cs | 16 +- 26 files changed, 429 insertions(+), 211 deletions(-) create mode 100644 src/Microsoft.AspNet.Mvc.Rendering/View/DynamicViewData.cs delete mode 100644 src/Microsoft.AspNet.Mvc.Rendering/View/ViewData.cs create mode 100644 src/Microsoft.AspNet.Mvc.Rendering/View/ViewDataDictionary.cs rename src/Microsoft.AspNet.Mvc.Rendering/View/{ViewDataOfTModel.cs => ViewDataDictionaryOfT.cs} (72%) create mode 100644 test/Microsoft.AspNet.Mvc.Core.Test/ControllerTests.cs create mode 100644 test/Microsoft.AspNet.Mvc.Rendering.Test/ViewContextTests.cs diff --git a/samples/MvcSample.Web/Views/Home/Create.cshtml b/samples/MvcSample.Web/Views/Home/Create.cshtml index a0753ee117..6f71264af2 100644 --- a/samples/MvcSample.Web/Views/Home/Create.cshtml +++ b/samples/MvcSample.Web/Views/Home/Create.cshtml @@ -1,24 +1,23 @@ @using MvcSample.Web.Models @model User @{ - string nullValue = null; ViewBag.Title = (Model == null) ? "Create Page" : "Edit Page"; - if (ViewData["Gift"] == null) + if (ViewBag.Gift == null) { ViewBag.Gift = "nothing"; } }
-

@ViewBag.Title

-

Thanks for @ViewBag.Gift

+

@ViewBag.Title

+

Thanks for @ViewBag.Gift

@if (Model == null) { -

Howdy, your model is null.

+

Howdy, your model is null.

} else { -

Hello @Model.Name! Happy @Model.Age birthday.

+

Hello @Model.Name! Happy @(Model.Age)th birthday.

} @{ diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResultHelper.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResultHelper.cs index 62c5968246..d15a2dc6e7 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResultHelper.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResultHelper.cs @@ -36,7 +36,7 @@ namespace Microsoft.AspNet.Mvc return new JsonResult(value); } - public IActionResult View(string view, ViewData viewData) + public IActionResult View(string view, ViewDataDictionary viewData) { return new ViewResult(_serviceProvider, _viewEngine) { diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/ViewResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/ViewResult.cs index d1629af2cc..b2b16cfceb 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/ViewResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/ViewResult.cs @@ -21,7 +21,7 @@ namespace Microsoft.AspNet.Mvc public string ViewName {get; set; } - public ViewData ViewData { get; set; } + public ViewDataDictionary ViewData { get; set; } public async Task ExecuteResultAsync([NotNull] ActionContext context) { diff --git a/src/Microsoft.AspNet.Mvc.Core/Controller.cs b/src/Microsoft.AspNet.Mvc.Core/Controller.cs index ff36d217f6..a43e2e2655 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Controller.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Controller.cs @@ -6,6 +6,8 @@ namespace Microsoft.AspNet.Mvc { public class Controller { + private DynamicViewData _viewBag; + public void Initialize(IActionResultHelper actionResultHelper) { Result = actionResultHelper; @@ -33,11 +35,19 @@ namespace Microsoft.AspNet.Mvc public IUrlHelper Url { get; set; } - public ViewData ViewData { get; set; } + public ViewDataDictionary ViewData { get; set; } public dynamic ViewBag { - get { return ViewData; } + get + { + if (_viewBag == null) + { + _viewBag = new DynamicViewData(() => ViewData); + } + + return _viewBag; + } } public IActionResult View() diff --git a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerFactory.cs b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerFactory.cs index 1546fb8c51..dbfd211ff2 100644 --- a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerFactory.cs +++ b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerFactory.cs @@ -52,15 +52,19 @@ namespace Microsoft.AspNet.Mvc foreach (var prop in controllerType.GetRuntimeProperties()) { - if(prop.Name == "ActionContext" && prop.PropertyType.GetTypeInfo().IsAssignableFrom(typeof(ActionContext).GetTypeInfo())) + if(prop.Name == "ActionContext" && + prop.PropertyType.GetTypeInfo().IsAssignableFrom(typeof(ActionContext).GetTypeInfo())) { prop.SetValue(controller, actionContext); } - else if (prop.Name == "ViewData" && prop.PropertyType.GetTypeInfo().IsAssignableFrom(typeof(ViewData).GetTypeInfo())) + else if (prop.Name == "ViewData" && + prop.PropertyType.GetTypeInfo().IsAssignableFrom(typeof(ViewDataDictionary).GetTypeInfo())) { - prop.SetValue(controller, new ViewData(_serviceProvider.GetService(), actionContext.ModelState)); + prop.SetValue(controller, new ViewDataDictionary( + _serviceProvider.GetService(), actionContext.ModelState)); } - else if (prop.Name == "Url" && prop.PropertyType.GetTypeInfo().IsAssignableFrom(typeof(IUrlHelper).GetTypeInfo())) + 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/IActionResultHelper.cs b/src/Microsoft.AspNet.Mvc.Core/IActionResultHelper.cs index c0bf5c78a1..40dd5de1da 100644 --- a/src/Microsoft.AspNet.Mvc.Core/IActionResultHelper.cs +++ b/src/Microsoft.AspNet.Mvc.Core/IActionResultHelper.cs @@ -7,6 +7,6 @@ namespace Microsoft.AspNet.Mvc IActionResult Content(string value); IActionResult Content(string value, string contentType); IJsonResult Json(object value); - IActionResult View(string view, ViewData viewData); + IActionResult View(string view, ViewDataDictionary viewData); } } diff --git a/src/Microsoft.AspNet.Mvc.Core/ViewComponents/DefaultViewComponentInvoker.cs b/src/Microsoft.AspNet.Mvc.Core/ViewComponents/DefaultViewComponentInvoker.cs index b9c485731c..595266f6d9 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ViewComponents/DefaultViewComponentInvoker.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ViewComponents/DefaultViewComponentInvoker.cs @@ -83,10 +83,10 @@ namespace Microsoft.AspNet.Mvc prop.SetValue(component, context); } else if (prop.Name == "ViewData" && - typeof(ViewData).GetTypeInfo().IsAssignableFrom(prop.PropertyType.GetTypeInfo())) + typeof(ViewDataDictionary).GetTypeInfo().IsAssignableFrom(prop.PropertyType.GetTypeInfo())) { // We're flowing the viewbag across, but the concept of model doesn't really apply here - var viewData = new ViewData(context.ViewData); + var viewData = new ViewDataDictionary(context.ViewData); viewData.Model = null; prop.SetValue(component, viewData); diff --git a/src/Microsoft.AspNet.Mvc.Core/ViewComponents/DefaultViewComponentResultHelper.cs b/src/Microsoft.AspNet.Mvc.Core/ViewComponents/DefaultViewComponentResultHelper.cs index 45db71b0f0..fbe033623d 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ViewComponents/DefaultViewComponentResultHelper.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ViewComponents/DefaultViewComponentResultHelper.cs @@ -23,7 +23,7 @@ namespace Microsoft.AspNet.Mvc return new JsonViewComponentResult(value); } - public IViewComponentResult View([NotNull] string viewName, [NotNull] ViewData viewData) + public IViewComponentResult View([NotNull] string viewName, [NotNull] ViewDataDictionary viewData) { return new ViewViewComponentResult(_viewEngine, viewName, viewData); } diff --git a/src/Microsoft.AspNet.Mvc.Core/ViewComponents/IViewComponentResultHelper.cs b/src/Microsoft.AspNet.Mvc.Core/ViewComponents/IViewComponentResultHelper.cs index df283d33c3..98f92cbc5a 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ViewComponents/IViewComponentResultHelper.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ViewComponents/IViewComponentResultHelper.cs @@ -9,6 +9,6 @@ namespace Microsoft.AspNet.Mvc IViewComponentResult Json([NotNull] object value); - IViewComponentResult View([NotNull] string viewName, [NotNull] ViewData viewData); + IViewComponentResult View([NotNull] string viewName, [NotNull] ViewDataDictionary viewData); } } diff --git a/src/Microsoft.AspNet.Mvc.Core/ViewComponents/ViewComponent.cs b/src/Microsoft.AspNet.Mvc.Core/ViewComponents/ViewComponent.cs index f971a42f90..b8f9308adc 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ViewComponents/ViewComponent.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ViewComponents/ViewComponent.cs @@ -16,7 +16,7 @@ namespace Microsoft.AspNet.Mvc public ViewContext ViewContext { get; set; } - public ViewData ViewData { get; set; } + public ViewDataDictionary ViewData { get; set; } public void Initialize(IViewComponentResultHelper result) { @@ -40,7 +40,7 @@ namespace Microsoft.AspNet.Mvc public IViewComponentResult View(string viewName, TModel model) { - var viewData = new ViewData(ViewData); + var viewData = new ViewDataDictionary(ViewData); if (model != null) { viewData.Model = model; diff --git a/src/Microsoft.AspNet.Mvc.Core/ViewComponents/ViewViewComponentResult.cs b/src/Microsoft.AspNet.Mvc.Core/ViewComponents/ViewViewComponentResult.cs index 30c4391ea1..fb0f8b2542 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ViewComponents/ViewViewComponentResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ViewComponents/ViewViewComponentResult.cs @@ -14,9 +14,10 @@ namespace Microsoft.AspNet.Mvc private readonly IViewEngine _viewEngine; private readonly string _viewName; - private readonly ViewData _viewData; + private readonly ViewDataDictionary _viewData; - public ViewViewComponentResult([NotNull] IViewEngine viewEngine, [NotNull] string viewName, ViewData viewData) + public ViewViewComponentResult([NotNull] IViewEngine viewEngine, [NotNull] string viewName, + ViewDataDictionary viewData) { _viewEngine = viewEngine; _viewName = viewName; diff --git a/src/Microsoft.AspNet.Mvc.Razor/RazorView.cs b/src/Microsoft.AspNet.Mvc.Razor/RazorView.cs index 0c019660b6..849bc80181 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/RazorView.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/RazorView.cs @@ -26,6 +26,14 @@ namespace Microsoft.AspNet.Mvc.Razor get { return Context == null ? null : Context.Url; } } + public dynamic ViewBag + { + get + { + return (Context == null) ? null : Context.ViewBag; + } + } + private string BodyContent { get; set; } public virtual async Task RenderAsync([NotNull] ViewContext context) diff --git a/src/Microsoft.AspNet.Mvc.Razor/RazorViewOfT.cs b/src/Microsoft.AspNet.Mvc.Razor/RazorViewOfT.cs index a57df9b5e4..5893ab84e6 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/RazorViewOfT.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/RazorViewOfT.cs @@ -16,31 +16,26 @@ namespace Microsoft.AspNet.Mvc.Razor } } - public dynamic ViewBag - { - get { return ViewData; } - } - - public ViewData ViewData { get; private set; } + public ViewDataDictionary ViewData { get; private set; } public HtmlHelper Html { get; set; } public override Task RenderAsync([NotNull] ViewContext context) { - ViewData = context.ViewData as ViewData; + ViewData = context.ViewData as ViewDataDictionary; if (ViewData == null) { if (context.ViewData != null) { - ViewData = new ViewData(context.ViewData); + ViewData = new ViewDataDictionary(context.ViewData); } else { var metadataProvider = context.ServiceProvider.GetService(); - ViewData = new ViewData(metadataProvider); + ViewData = new ViewDataDictionary(metadataProvider); } - // Have new ViewData; make sure it's visible everywhere. + // Have new ViewDataDictionary; make sure it's visible everywhere. context.ViewData = ViewData; } diff --git a/src/Microsoft.AspNet.Mvc.Rendering/Html/HtmlHelper.cs b/src/Microsoft.AspNet.Mvc.Rendering/Html/HtmlHelper.cs index 99ba0701f3..f9d2600281 100644 --- a/src/Microsoft.AspNet.Mvc.Rendering/Html/HtmlHelper.cs +++ b/src/Microsoft.AspNet.Mvc.Rendering/Html/HtmlHelper.cs @@ -17,7 +17,7 @@ namespace Microsoft.AspNet.Mvc.Rendering public static readonly string ValidationSummaryCssClassName = "validation-summary-errors"; public static readonly string ValidationSummaryValidCssClassName = "validation-summary-valid"; - public HtmlHelper([NotNull] HttpContext httpContext, ViewData viewData) + public HtmlHelper([NotNull] HttpContext httpContext, ViewDataDictionary viewData) { HttpContext = httpContext; ViewData = viewData; @@ -30,7 +30,7 @@ namespace Microsoft.AspNet.Mvc.Rendering public HttpContext HttpContext { get; private set; } - public ViewData ViewData + public ViewDataDictionary ViewData { get; private set; diff --git a/src/Microsoft.AspNet.Mvc.Rendering/Html/HtmlHelperOfT.cs b/src/Microsoft.AspNet.Mvc.Rendering/Html/HtmlHelperOfT.cs index 094ccd1d48..17041c3326 100644 --- a/src/Microsoft.AspNet.Mvc.Rendering/Html/HtmlHelperOfT.cs +++ b/src/Microsoft.AspNet.Mvc.Rendering/Html/HtmlHelperOfT.cs @@ -4,13 +4,13 @@ namespace Microsoft.AspNet.Mvc.Rendering { public class HtmlHelper : HtmlHelper { - public HtmlHelper([NotNull]HttpContext httpContext, ViewData viewData) + public HtmlHelper([NotNull]HttpContext httpContext, ViewDataDictionary viewData) : base(httpContext, viewData) { ViewData = viewData; } - public new ViewData ViewData + public new ViewDataDictionary ViewData { get; private set; } diff --git a/src/Microsoft.AspNet.Mvc.Rendering/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Mvc.Rendering/Properties/Resources.Designer.cs index 48fade35b3..26d6012253 100644 --- a/src/Microsoft.AspNet.Mvc.Rendering/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Mvc.Rendering/Properties/Resources.Designer.cs @@ -42,6 +42,22 @@ namespace Microsoft.AspNet.Mvc.Rendering return string.Format(CultureInfo.CurrentCulture, GetString("Common_PartialViewNotFound"), p0, p1); } + /// + /// ViewData value must not be null. + /// + internal static string DynamicViewData_ViewDataNull + { + get { return GetString("DynamicViewData_ViewDataNull"); } + } + + /// + /// ViewData value must not be null. + /// + internal static string FormatDynamicViewData_ViewDataNull() + { + return GetString("DynamicViewData_ViewDataNull"); + } + /// /// The model item passed is null, but this ViewData instance requires a non-null model item of type '{0}'. /// diff --git a/src/Microsoft.AspNet.Mvc.Rendering/Properties/Resources.resx b/src/Microsoft.AspNet.Mvc.Rendering/Properties/Resources.resx index b47862e0d4..ac76688ab9 100644 --- a/src/Microsoft.AspNet.Mvc.Rendering/Properties/Resources.resx +++ b/src/Microsoft.AspNet.Mvc.Rendering/Properties/Resources.resx @@ -123,11 +123,14 @@ The partial view '{0}' was not found or no view engine supports the searched locations. The following locations were searched:{1} + + ViewData value must not be null. + - The model item passed is null, but this ViewData instance requires a non-null model item of type '{0}'. + The model item passed is null, but this ViewDataDictionary instance requires a non-null model item of type '{0}'. - The model item passed into the ViewData is of type '{0}', but this ViewData instance requires a model item of type '{1}'. + The model item passed into the ViewDataDictionary is of type '{0}', but this ViewDataDictionary instance requires a model item of type '{1}'. The view '{0}' was not found. The following locations were searched:{1}. diff --git a/src/Microsoft.AspNet.Mvc.Rendering/View/DynamicViewData.cs b/src/Microsoft.AspNet.Mvc.Rendering/View/DynamicViewData.cs new file mode 100644 index 0000000000..11f97253ca --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Rendering/View/DynamicViewData.cs @@ -0,0 +1,55 @@ +using System; +using System.Collections.Generic; +using System.Dynamic; + +namespace Microsoft.AspNet.Mvc.Rendering +{ + public class DynamicViewData : DynamicObject + { + private readonly Func _viewDataFunc; + + public DynamicViewData([NotNull] Func viewDataFunc) + { + _viewDataFunc = viewDataFunc; + } + + private ViewDataDictionary ViewData + { + get + { + ViewDataDictionary viewData = _viewDataFunc(); + if (viewData == null) + { + throw new InvalidOperationException(Resources.DynamicViewData_ViewDataNull); + } + + return viewData; + } + } + + // Implementing this function extends the ViewBag contract, supporting or improving some scenarios. For example + // having this method improves the debugging experience as it provides the debugger with the list of all + // properties currently defined on the object. + public override IEnumerable GetDynamicMemberNames() + { + return ViewData.Keys; + } + + public override bool TryGetMember([NotNull] GetMemberBinder binder, out object result) + { + result = ViewData[binder.Name]; + + // ViewDataDictionary[key] will never throw a KeyNotFoundException. + // Similarly, return true so caller does not throw. + return true; + } + + public override bool TrySetMember([NotNull] SetMemberBinder binder, object value) + { + ViewData[binder.Name] = value; + + // Can always add / update a ViewDataDictionary value. + return true; + } + } +} diff --git a/src/Microsoft.AspNet.Mvc.Rendering/View/ViewContext.cs b/src/Microsoft.AspNet.Mvc.Rendering/View/ViewContext.cs index 7939753976..2459650222 100644 --- a/src/Microsoft.AspNet.Mvc.Rendering/View/ViewContext.cs +++ b/src/Microsoft.AspNet.Mvc.Rendering/View/ViewContext.cs @@ -7,6 +7,8 @@ namespace Microsoft.AspNet.Mvc.Rendering { public class ViewContext { + private DynamicViewData _viewBag; + public ViewContext(IServiceProvider serviceProvider, HttpContext httpContext, IDictionary viewEngineContext) { ServiceProvider = serviceProvider; @@ -22,7 +24,20 @@ namespace Microsoft.AspNet.Mvc.Rendering public IUrlHelper Url { get; set; } - public ViewData ViewData { get; set; } + public dynamic ViewBag + { + get + { + if (_viewBag == null) + { + _viewBag = new DynamicViewData(() => ViewData); + } + + return _viewBag; + } + } + + public ViewDataDictionary ViewData { get; set; } public IDictionary ViewEngineContext { get; private set; } diff --git a/src/Microsoft.AspNet.Mvc.Rendering/View/ViewData.cs b/src/Microsoft.AspNet.Mvc.Rendering/View/ViewData.cs deleted file mode 100644 index 4752b008a5..0000000000 --- a/src/Microsoft.AspNet.Mvc.Rendering/View/ViewData.cs +++ /dev/null @@ -1,155 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Dynamic; -using Microsoft.AspNet.Mvc.ModelBinding; - -namespace Microsoft.AspNet.Mvc.Rendering -{ - public class ViewData : DynamicObject - { - private readonly Dictionary _data; - private object _model; - private ModelMetadata _modelMetadata; - 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) - { - _modelMetadata = source.ModelMetadata; - - 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); - } - - public object Model - { - get { return _model; } - set { SetModel(value); } - } - - public ModelStateDictionary ModelState { get; private set; } - - public dynamic this[string index] - { - get - { - dynamic result; - - _data.TryGetValue(index, out result); - - return result; - } - set - { - _data[index] = value; - } - } - - public virtual ModelMetadata ModelMetadata - { - get - { - return _modelMetadata; - } - set - { - _modelMetadata = value; - } - } - - /// - /// Provider for subclasses that need it to override . - /// - protected IModelMetadataProvider MetadataProvider - { - get { return _metadataProvider; } - } - - public Dictionary.Enumerator GetEnumerator() - { - return _data.GetEnumerator(); - } - - public override bool TryGetMember(GetMemberBinder binder, out object result) - { - result = _data[binder.Name]; - - // We return true here because ViewDataDictionary returns null if the key is not - // in the dictionary, so we simply pass on the returned value. - return true; - } - - public override bool TrySetMember(SetMemberBinder binder, object value) - { - // This cast should always succeed. - dynamic v = value; - _data[binder.Name] = v; - return true; - } - - public override bool TryGetIndex(GetIndexBinder binder, object[] indexes, out object result) - { - if (indexes == null || indexes.Length != 1) - { - throw new ArgumentException("Invalid number of indexes"); - } - - object index = indexes[0]; - result = this[(string)index]; - return true; - } - - public override bool TrySetIndex(SetIndexBinder binder, object[] indexes, object value) - { - if (indexes == null || indexes.Length != 1) - { - throw new ArgumentException("Invalid number of indexes"); - } - - object index = indexes[0]; - - // This cast should always succeed. - this[(string)index] = value; - return true; - } - - // This method will execute before the derived type's instance constructor executes. Derived types must - // be aware of this and should plan accordingly. For example, the logic in SetModel() should be simple - // enough so as not to depend on the "this" pointer referencing a fully constructed object. - protected virtual void SetModel(object value) - { - _model = value; - if (value == null) - { - // Unable to determine model metadata. - _modelMetadata = null; - } - else if (_modelMetadata == null || value.GetType() != ModelMetadata.ModelType) - { - // Reset or override model metadata based on new value type. - _modelMetadata = _metadataProvider.GetMetadataForType(() => value, value.GetType()); - } - } - } -} diff --git a/src/Microsoft.AspNet.Mvc.Rendering/View/ViewDataDictionary.cs b/src/Microsoft.AspNet.Mvc.Rendering/View/ViewDataDictionary.cs new file mode 100644 index 0000000000..8de4584369 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Rendering/View/ViewDataDictionary.cs @@ -0,0 +1,198 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using Microsoft.AspNet.Mvc.ModelBinding; + +namespace Microsoft.AspNet.Mvc.Rendering +{ + public class ViewDataDictionary : IDictionary + { + private readonly IDictionary _data; + private object _model; + private ModelMetadata _modelMetadata; + private IModelMetadataProvider _metadataProvider; + + public ViewDataDictionary([NotNull] IModelMetadataProvider metadataProvider) + : this(metadataProvider, new ModelStateDictionary()) + { + } + + public ViewDataDictionary([NotNull] IModelMetadataProvider metadataProvider, + [NotNull] ModelStateDictionary modelState) + { + ModelState = modelState; + _data = new Dictionary(StringComparer.OrdinalIgnoreCase); + _metadataProvider = metadataProvider; + } + + /// + /// copy constructor for use when model type does not change. + /// + public ViewDataDictionary([NotNull] ViewDataDictionary source) + : this(source, source.Model) + { + } + + /// + /// copy constructor for use when model type may change. This avoids + /// exceptions a derived class may throw when is called. + /// + public ViewDataDictionary([NotNull] ViewDataDictionary source, object model) + : this(source.MetadataProvider) + { + _modelMetadata = source.ModelMetadata; + + foreach (var entry in source.ModelState) + { + ModelState.Add(entry.Key, entry.Value); + } + + foreach (var entry in source) + { + _data.Add(entry.Key, entry.Value); + } + + SetModel(model); + } + + public object Model + { + get { return _model; } + set { SetModel(value); } + } + + public ModelStateDictionary ModelState { get; private set; } + + public virtual ModelMetadata ModelMetadata + { + get + { + return _modelMetadata; + } + set + { + _modelMetadata = value; + } + } + + /// + /// Provider for subclasses that need it to override . + /// + protected IModelMetadataProvider MetadataProvider + { + get { return _metadataProvider; } + } + + #region IDictionary properties + // Do not just pass through to _data: Indexer should not throw a KeyNotFoundException. + public object this[string index] + { + get + { + object result; + _data.TryGetValue(index, out result); + return result; + } + set + { + _data[index] = value; + } + } + + public int Count + { + get { return _data.Count; } + } + + public bool IsReadOnly + { + get { return _data.IsReadOnly; } + } + + public ICollection Keys + { + get { return _data.Keys; } + } + + public ICollection Values + { + get { return _data.Values; } + } + #endregion + + // This method will execute before the derived type's instance constructor executes. Derived types must + // be aware of this and should plan accordingly. For example, the logic in SetModel() should be simple + // enough so as not to depend on the "this" pointer referencing a fully constructed object. + protected virtual void SetModel(object value) + { + _model = value; + if (value == null) + { + // Unable to determine model metadata. + _modelMetadata = null; + } + else if (_modelMetadata == null || value.GetType() != ModelMetadata.ModelType) + { + // Reset or override model metadata based on new value type. + _modelMetadata = _metadataProvider.GetMetadataForType(() => value, value.GetType()); + } + } + + #region IDictionary methods + public void Add([NotNull] string key, object value) + { + _data.Add(key, value); + } + + public bool ContainsKey([NotNull] string key) + { + return _data.ContainsKey(key); + } + + public bool Remove([NotNull] string key) + { + return _data.Remove(key); + } + + public bool TryGetValue([NotNull] string key, out object value) + { + return _data.TryGetValue(key, out value); + } + + public void Add([NotNull] KeyValuePair item) + { + _data.Add(item); + } + + public void Clear() + { + _data.Clear(); + } + + public bool Contains([NotNull] KeyValuePair item) + { + return _data.Contains(item); + } + + public void CopyTo([NotNull] KeyValuePair[] array, int arrayIndex) + { + _data.CopyTo(array, arrayIndex); + } + + public bool Remove([NotNull] KeyValuePair item) + { + return _data.Remove(item); + } + + IEnumerator> IEnumerable>.GetEnumerator() + { + return _data.GetEnumerator(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return _data.GetEnumerator(); + } + #endregion + } +} diff --git a/src/Microsoft.AspNet.Mvc.Rendering/View/ViewDataOfTModel.cs b/src/Microsoft.AspNet.Mvc.Rendering/View/ViewDataDictionaryOfT.cs similarity index 72% rename from src/Microsoft.AspNet.Mvc.Rendering/View/ViewDataOfTModel.cs rename to src/Microsoft.AspNet.Mvc.Rendering/View/ViewDataDictionaryOfT.cs index 6c658da44e..46abcb56ee 100644 --- a/src/Microsoft.AspNet.Mvc.Rendering/View/ViewDataOfTModel.cs +++ b/src/Microsoft.AspNet.Mvc.Rendering/View/ViewDataDictionaryOfT.cs @@ -3,27 +3,35 @@ using Microsoft.AspNet.Mvc.ModelBinding; namespace Microsoft.AspNet.Mvc.Rendering { - public class ViewData : ViewData + public class ViewDataDictionary : ViewDataDictionary { - // Fallback ModelMetadata based on TModel. Used when Model is null and base ViewData class is unable to - // determine the correct metadata. + // Fallback ModelMetadata based on TModel. Used when Model is null and base ViewDataDictionary class is unable + // to determine the correct metadata. private readonly ModelMetadata _defaultModelMetadata; - public ViewData([NotNull] IModelMetadataProvider metadataProvider) + public ViewDataDictionary([NotNull] IModelMetadataProvider metadataProvider) : base(metadataProvider) { _defaultModelMetadata = MetadataProvider.GetMetadataForType(null, typeof(TModel)); } - public ViewData([NotNull] IModelMetadataProvider metadataProvider, [NotNull] ModelStateDictionary modelState) + public ViewDataDictionary([NotNull] IModelMetadataProvider metadataProvider, + [NotNull] ModelStateDictionary modelState) : base(metadataProvider, modelState) { } - public ViewData(ViewData source) - : base(source) + /// + public ViewDataDictionary([NotNull] ViewDataDictionary source) + : this(source, source.Model) { - var original = source as ViewData; + } + + /// + public ViewDataDictionary([NotNull] ViewDataDictionary source, object model) + : base(source, model) + { + var original = source as ViewDataDictionary; if (original != null) { _defaultModelMetadata = original._defaultModelMetadata; diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerTests.cs new file mode 100644 index 0000000000..25cf8e4432 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerTests.cs @@ -0,0 +1,31 @@ +using Microsoft.AspNet.Mvc.ModelBinding; +using Microsoft.AspNet.Mvc.Rendering; +using Xunit; + +namespace Microsoft.AspNet.Mvc.Core +{ + public class ControllerTests + { + [Fact] + public void SettingViewData_AlsoUpdatesViewBag() + { + // Arrange + var metadataProvider = new DataAnnotationsModelMetadataProvider(); + var controller = new Controller(); + var originalViewData = controller.ViewData = new ViewDataDictionary(metadataProvider); + var replacementViewData = new ViewDataDictionary(metadataProvider); + + // Act + controller.ViewBag.Hello = "goodbye"; + controller.ViewData = replacementViewData; + controller.ViewBag.Another = "property"; + + // Assert + Assert.NotSame(originalViewData, controller.ViewData); + Assert.Same(replacementViewData, controller.ViewData); + Assert.Null(controller.ViewBag.Hello); + Assert.Equal("property", controller.ViewBag.Another); + Assert.Equal("property", controller.ViewData["Another"]); + } + } +} diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/project.json b/test/Microsoft.AspNet.Mvc.Core.Test/project.json index 1dffea98f2..3acb6991c7 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/project.json +++ b/test/Microsoft.AspNet.Mvc.Core.Test/project.json @@ -4,6 +4,8 @@ "Microsoft.AspNet.Mvc.Core" : "", "Microsoft.AspNet.Mvc" : "", "Microsoft.AspNet.Testing": "0.1-alpha-*", + "Microsoft.AspNet.Mvc.ModelBinding": "", + "Microsoft.AspNet.Mvc.Rendering": "", "Moq": "4.2.1312.1622", "Xunit.KRunner": "0.1-alpha-*", "xunit.abstractions": "2.0.0-aspnet-*", diff --git a/test/Microsoft.AspNet.Mvc.Rendering.Test/ViewContextTests.cs b/test/Microsoft.AspNet.Mvc.Rendering.Test/ViewContextTests.cs new file mode 100644 index 0000000000..35fe3771ed --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Rendering.Test/ViewContextTests.cs @@ -0,0 +1,28 @@ +using Xunit; + +namespace Microsoft.AspNet.Mvc.Rendering +{ + public class ViewContextTests + { + [Fact] + public void SettingViewData_AlsoUpdatesViewBag() + { + // Arrange (eventually passing null to these consturctors will throw) + var context = new ViewContext(serviceProvider: null, httpContext: null, viewEngineContext: null); + var originalViewData = context.ViewData = new ViewDataDictionary(metadataProvider: null); + var replacementViewData = new ViewDataDictionary(metadataProvider: null); + + // Act + context.ViewBag.Hello = "goodbye"; + context.ViewData = replacementViewData; + context.ViewBag.Another = "property"; + + // Assert + Assert.NotSame(originalViewData, context.ViewData); + Assert.Same(replacementViewData, context.ViewData); + Assert.Null(context.ViewBag.Hello); + Assert.Equal("property", context.ViewBag.Another); + Assert.Equal("property", context.ViewData["Another"]); + } + } +} diff --git a/test/Microsoft.AspNet.Mvc.Rendering.Test/ViewDataOfTTest.cs b/test/Microsoft.AspNet.Mvc.Rendering.Test/ViewDataOfTTest.cs index aaba444b23..218a9c068b 100644 --- a/test/Microsoft.AspNet.Mvc.Rendering.Test/ViewDataOfTTest.cs +++ b/test/Microsoft.AspNet.Mvc.Rendering.Test/ViewDataOfTTest.cs @@ -10,24 +10,24 @@ namespace Microsoft.AspNet.Mvc.Rendering public void SettingModelThrowsIfTheModelIsNull() { // Arrange - var viewDataOfT = new ViewData(new DataAnnotationsModelMetadataProvider()); - ViewData viewData = viewDataOfT; + var viewDataOfT = new ViewDataDictionary(new DataAnnotationsModelMetadataProvider()); + ViewDataDictionary viewData = viewDataOfT; // Act and Assert Exception ex = Assert.Throws(() => viewData.Model = null); - Assert.Equal("The model item passed is null, but this ViewData instance requires a non-null model item of type 'System.Int32'.", ex.Message); + Assert.Equal("The model item passed is null, but this ViewDataDictionary instance requires a non-null model item of type 'System.Int32'.", ex.Message); } [Fact] public void SettingModelThrowsIfTheModelIsIncompatible() { // Arrange - var viewDataOfT = new ViewData(new DataAnnotationsModelMetadataProvider()); - ViewData viewData = viewDataOfT; + var viewDataOfT = new ViewDataDictionary(new DataAnnotationsModelMetadataProvider()); + ViewDataDictionary viewData = viewDataOfT; // Act and Assert Exception ex = Assert.Throws(() => viewData.Model = DateTime.UtcNow); - Assert.Equal("The model item passed into the ViewData is of type 'System.DateTime', but this ViewData instance requires a model item of type 'System.String'.", ex.Message); + Assert.Equal("The model item passed into the ViewDataDictionary is of type 'System.DateTime', but this ViewDataDictionary instance requires a model item of type 'System.String'.", ex.Message); } [Fact] @@ -35,8 +35,8 @@ namespace Microsoft.AspNet.Mvc.Rendering { // Arrange string value = "some value"; - var viewDataOfT = new ViewData(new DataAnnotationsModelMetadataProvider()); - ViewData viewData = viewDataOfT; + var viewDataOfT = new ViewDataDictionary(new DataAnnotationsModelMetadataProvider()); + ViewDataDictionary viewData = viewDataOfT; // Act viewData.Model = value;