From 653d31b336efbad0568c8305907e52fbccdff200 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Mon, 27 Oct 2014 16:14:32 -0700 Subject: [PATCH 1/4] Don't lock metadata references --- .../Compilation/RoslynCompilationService.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.AspNet.Mvc.Razor/Compilation/RoslynCompilationService.cs b/src/Microsoft.AspNet.Mvc.Razor/Compilation/RoslynCompilationService.cs index c60429e34d..9d0df4a6ad 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/Compilation/RoslynCompilationService.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/Compilation/RoslynCompilationService.cs @@ -7,6 +7,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; using System.Reflection; +using System.Reflection.PortableExecutable; using System.Runtime.InteropServices; using Microsoft.AspNet.FileSystems; using Microsoft.CodeAnalysis; @@ -174,7 +175,11 @@ namespace Microsoft.AspNet.Mvc.Razor.Compilation { var metadata = _metadataFileCache.GetOrAdd(path, _ => { - return AssemblyMetadata.CreateFromStream(File.OpenRead(path)); + using (var stream = File.OpenRead(path)) + { + var moduleMetadata = ModuleMetadata.CreateFromStream(stream, PEStreamOptions.PrefetchMetadata); + return AssemblyMetadata.Create(moduleMetadata); + } }); return metadata.GetReference(); From 8d2a1c47e5e620deb6ab3f7ed3c806582bb8639e Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Wed, 22 Oct 2014 22:13:03 -0700 Subject: [PATCH 2/4] Change `ViewDataDictionary` copy constructor to ensure `ModelMetadata` is never that for `object` - `ViewDataDictionary.ModelMetadata` was for `object` after base copy constructor got value from `ViewDataDictionary` - problem led to #1426 symptoms - with copy constructor leaving `base.ModelMetadata==null` more often, `ViewDataDictionary.ModelMetadata` usually tracks `TModel` if `Model==null` nit: - fix existing comment in main `ViewDataDictionary` copy constructor --- .../ViewDataDictionary.cs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/ViewDataDictionary.cs b/src/Microsoft.AspNet.Mvc.Core/ViewDataDictionary.cs index 6b940b6e94..eca162da51 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ViewDataDictionary.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ViewDataDictionary.cs @@ -33,7 +33,8 @@ namespace Microsoft.AspNet.Mvc } /// - /// copy constructor for use when model type does not change. + /// copy constructor for use when model type does not change or caller will + /// immediately set the property. /// public ViewDataDictionary([NotNull] ViewDataDictionary source) : this(source, source.Model) @@ -50,9 +51,15 @@ namespace Microsoft.AspNet.Mvc new CopyOnWriteDictionary(source, StringComparer.OrdinalIgnoreCase), new TemplateInfo(source.TemplateInfo)) { - _modelMetadata = source.ModelMetadata; - // If we're constructing a derived ViewDataDictionary (or any other value type), - // SetModel will throw if we try to set it to null. We should not throw in that case. + // Avoid copying information about the object type. To do so when model==null would confuse the + // ViewDataDictionary.ModelMetadata getter. + if (source.ModelMetadata?.ModelType != typeof(object)) + { + _modelMetadata = source.ModelMetadata; + } + + // If we're constructing a derived ViewDataDictionary where TModel is a non-Nullable value type, + // SetModel will throw if we try to call it with null. We should not throw in that case. if (model != null) { SetModel(model); From 4bde6f6caf2e3f9c9f1bdaed3b0ae3b4ddefa487 Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Wed, 22 Oct 2014 22:56:28 -0700 Subject: [PATCH 3/4] Add "/home/nulluser" view to MVC sample - exercises display and editor helpers when `Model==null` --- samples/MvcSample.Web/HomeController.cs | 5 +++ .../MvcSample.Web/Views/Home/NullUser.cshtml | 42 +++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 samples/MvcSample.Web/Views/Home/NullUser.cshtml diff --git a/samples/MvcSample.Web/HomeController.cs b/samples/MvcSample.Web/HomeController.cs index 6bf0088cd8..07f1863756 100644 --- a/samples/MvcSample.Web/HomeController.cs +++ b/samples/MvcSample.Web/HomeController.cs @@ -17,6 +17,11 @@ namespace MvcSample.Web return View("MyView", CreateUser()); } + public IActionResult NullUser() + { + return View(); + } + public ActionResult ValidationSummary() { ModelState.AddModelError("something", "Something happened, show up in validation summary."); diff --git a/samples/MvcSample.Web/Views/Home/NullUser.cshtml b/samples/MvcSample.Web/Views/Home/NullUser.cshtml new file mode 100644 index 0000000000..fdd8a4d8ca --- /dev/null +++ b/samples/MvcSample.Web/Views/Home/NullUser.cshtml @@ -0,0 +1,42 @@ +@* + For more information on enabling MVC for empty projects, visit http://go.microsoft.com/fwlink/?LinkID=397860 +*@ +@{ + // ViewBag.Title = "Home Page"; +} + +@using MvcSample.Web.Models +@model User + +
+
+ EditorForModel: + @Html.EditorForModel() +
+
+ Editor of empty string: + @Html.Editor("") +
+
+ EditorFor of identity expression: + @Html.EditorFor(m => m) +
+ +
+ DisplayForModel: + '@Html.DisplayForModel()' +
+ +
+ Display: + for name '@Html.Display("Name")' + and alive '@Html.Display("Alive")' +
+ +
+ Editor: + for name @Html.Editor("Name") + alive @Html.Editor("Alive") + and alive expression @Html.EditorFor(m => m.Alive) +
+
\ No newline at end of file From cfcb1f2e2fec9d484ca0a32f054d699eb7c8a925 Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Wed, 22 Oct 2014 23:25:43 -0700 Subject: [PATCH 4/4] Add more unit tests of `ViewDataDictionary` copy constructors --- .../ViewDataDictionaryTest.cs | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ViewDataDictionaryTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ViewDataDictionaryTest.cs index 7223b235bd..c7472c4fd1 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ViewDataDictionaryTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ViewDataDictionaryTest.cs @@ -159,6 +159,87 @@ namespace Microsoft.AspNet.Mvc.Core Assert.IsType>(viewData.Data); } + [Theory] + [InlineData(typeof(int))] + [InlineData(typeof(string))] + [InlineData(typeof(IEnumerable))] + [InlineData(typeof(List))] + [InlineData(typeof(string[]))] + [InlineData(typeof(Dictionary))] + public void CopyConstructors_CopyModelMetadata(Type type) + { + // Arrange + var metadataProvider = new EmptyModelMetadataProvider(); + var metadata = metadataProvider.GetMetadataForType(() => null, type); + var source = new ViewDataDictionary(metadataProvider) + { + ModelMetadata = metadata, + }; + + // Act + var viewData1 = new ViewDataDictionary(source); + var viewData2 = new ViewDataDictionary(source, model: null); + + // Assert + Assert.Same(metadata, viewData1.ModelMetadata); + Assert.Same(metadata, viewData2.ModelMetadata); + } + + [Fact] + public void CopyConstructors_IgnoreModelMetadata_IfForTypeObject() + { + // Arrange + var metadataProvider = new EmptyModelMetadataProvider(); + var metadata = metadataProvider.GetMetadataForType(() => null, typeof(object)); + var source = new ViewDataDictionary(metadataProvider) + { + ModelMetadata = metadata, + }; + + // Act + var viewData1 = new ViewDataDictionary(source); + var viewData2 = new ViewDataDictionary(source, model: null); + + // Assert + Assert.Null(viewData1.ModelMetadata); + Assert.Null(viewData2.ModelMetadata); + } + + [Theory] + [InlineData(typeof(int), "test string", typeof(string))] + [InlineData(typeof(string), 23, typeof(int))] + [InlineData(typeof(IEnumerable), new[] { "1", "2", "3", }, typeof(object[]))] + [InlineData(typeof(List), new[] { 1, 2, 3, }, typeof(object[]))] + public void CopyConstructors_OverrideSourceMetadata_IfModelNonNull( + Type sourceType, + object instance, + Type expectedType) + { + // Arrange + var metadataProvider = new EmptyModelMetadataProvider(); + var metadata = metadataProvider.GetMetadataForType(() => null, sourceType); + var source = new ViewDataDictionary(metadataProvider) + { + ModelMetadata = metadata, + }; + + // Act + var viewData1 = new ViewDataDictionary(source) + { + Model = instance, + }; + var viewData2 = new ViewDataDictionary(source, model: instance); + + // Assert + Assert.NotNull(viewData1.ModelMetadata); + Assert.Equal(expectedType, viewData1.ModelMetadata.ModelType); + Assert.Equal(expectedType, viewData1.ModelMetadata.RealModelType); + + Assert.NotNull(viewData2.ModelMetadata); + Assert.Equal(expectedType, viewData2.ModelMetadata.ModelType); + Assert.Equal(expectedType, viewData2.ModelMetadata.RealModelType); + } + public static TheoryData Eval_EvaluatesExpressionsData { get