diff --git a/src/Microsoft.AspNet.Mvc.Common/CopyOnWriteDictionary.cs b/src/Microsoft.AspNet.Mvc.Common/CopyOnWriteDictionary.cs new file mode 100644 index 0000000000..716e9245ea --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Common/CopyOnWriteDictionary.cs @@ -0,0 +1,148 @@ +// Copyright (c) Microsoft Open Technologies, Inc. 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; +using System.Collections.Generic; + +namespace Microsoft.AspNet.Mvc +{ + /// + /// A that defers creating a shallow copy of the source dictionary until + /// a mutative operation has been performed on it. + /// + internal class CopyOnWriteDictionary : IDictionary + { + private readonly IDictionary _sourceDictionary; + private readonly IEqualityComparer _comparer; + private IDictionary _innerDictionary; + + public CopyOnWriteDictionary([NotNull] IDictionary sourceDictionary, + [NotNull] IEqualityComparer comparer) + { + _sourceDictionary = sourceDictionary; + _comparer = comparer; + } + + private IDictionary ReadDictionary + { + get + { + return _innerDictionary ?? _sourceDictionary; + } + } + + private IDictionary WriteDictionary + { + get + { + if (_innerDictionary == null) + { + _innerDictionary = new Dictionary(_sourceDictionary, + _comparer); + } + + return _innerDictionary; + } + } + + public virtual ICollection Keys + { + get + { + return ReadDictionary.Keys; + } + } + + public virtual ICollection Values + { + get + { + return ReadDictionary.Values; + } + } + + public virtual int Count + { + get + { + return ReadDictionary.Count; + } + } + + public virtual bool IsReadOnly + { + get + { + return false; + } + } + + public virtual TValue this[[NotNull] TKey key] + { + get + { + return ReadDictionary[key]; + } + set + { + WriteDictionary[key] = value; + } + } + + public virtual bool ContainsKey([NotNull] TKey key) + { + return ReadDictionary.ContainsKey(key); + } + + public virtual void Add([NotNull] TKey key, TValue value) + { + WriteDictionary.Add(key, value); + } + + public virtual bool Remove([NotNull] TKey key) + { + return WriteDictionary.Remove(key); + } + + public virtual bool TryGetValue([NotNull] TKey key, out TValue value) + { + return ReadDictionary.TryGetValue(key, out value); + } + + public virtual void Add(KeyValuePair item) + { + WriteDictionary.Add(item); + } + + public virtual void Clear() + { + WriteDictionary.Clear(); + } + + public virtual bool Contains(KeyValuePair item) + { + return ReadDictionary.Contains(item); + } + + public virtual void CopyTo([NotNull] KeyValuePair[] array, int arrayIndex) + { + ReadDictionary.CopyTo(array, arrayIndex); + } + + public bool Remove(KeyValuePair item) + { + return WriteDictionary.Remove(item); + } + + public virtual IEnumerator> GetEnumerator() + { + return ReadDictionary.GetEnumerator(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return GetEnumerator(); + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/ViewDataDictionary.cs b/src/Microsoft.AspNet.Mvc.Core/ViewDataDictionary.cs index 60cd085413..061952922d 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ViewDataDictionary.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ViewDataDictionary.cs @@ -24,12 +24,12 @@ namespace Microsoft.AspNet.Mvc } public ViewDataDictionary([NotNull] IModelMetadataProvider metadataProvider, - [NotNull] ModelStateDictionary modelState) + [NotNull] ModelStateDictionary modelState) + : this(metadataProvider, + modelState: modelState, + data: new Dictionary(StringComparer.OrdinalIgnoreCase), + templateInfo: new TemplateInfo()) { - ModelState = modelState; - TemplateInfo = new TemplateInfo(); - _data = new Dictionary(StringComparer.OrdinalIgnoreCase); - _metadataProvider = metadataProvider; } /// @@ -45,24 +45,26 @@ namespace Microsoft.AspNet.Mvc /// exceptions a derived class may throw when is called. /// public ViewDataDictionary([NotNull] ViewDataDictionary source, object model) - : this(source.MetadataProvider) + : this(source.MetadataProvider, + new ModelStateDictionary(source.ModelState), + new CopyOnWriteDictionary(source, StringComparer.OrdinalIgnoreCase), + new TemplateInfo(source.TemplateInfo)) { _modelMetadata = source.ModelMetadata; - TemplateInfo = new TemplateInfo(source.TemplateInfo); - - 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); } + private ViewDataDictionary(IModelMetadataProvider metadataProvider, + ModelStateDictionary modelState, + IDictionary data, + TemplateInfo templateInfo) + { + _metadataProvider = metadataProvider; + ModelState = modelState; + _data = data; + TemplateInfo = templateInfo; + } + public object Model { get { return _model; } @@ -130,6 +132,12 @@ namespace Microsoft.AspNet.Mvc } #endregion + // for unit testing + internal IDictionary Data + { + get { return _data; } + } + public object Eval(string expression) { var info = GetViewDataInfo(expression); diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/ModelStateDictionary.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelStateDictionary.cs index 42ec4596c5..57d248bfe4 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/ModelStateDictionary.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelStateDictionary.cs @@ -11,19 +11,17 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { public class ModelStateDictionary : IDictionary { - private readonly IDictionary _innerDictionary = - new Dictionary(StringComparer.OrdinalIgnoreCase); + private readonly IDictionary _innerDictionary; public ModelStateDictionary() { + _innerDictionary = new Dictionary(StringComparer.OrdinalIgnoreCase); } public ModelStateDictionary([NotNull] ModelStateDictionary dictionary) { - foreach (var entry in dictionary) - { - _innerDictionary.Add(entry.Key, entry.Value); - } + _innerDictionary = new CopyOnWriteDictionary(dictionary, + StringComparer.OrdinalIgnoreCase); } #region IDictionary properties @@ -76,6 +74,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } } + // For unit testing + internal IDictionary InnerDictionary + { + get { return _innerDictionary; } + } + public void AddModelError([NotNull] string key, [NotNull] Exception exception) { var modelState = GetModelStateForKey(key); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/CopyOnWriteDictionaryTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/CopyOnWriteDictionaryTest.cs new file mode 100644 index 0000000000..f1d7263235 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/CopyOnWriteDictionaryTest.cs @@ -0,0 +1,102 @@ +// Copyright (c) Microsoft Open Technologies, Inc. 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 Moq; +using Xunit; + +namespace Microsoft.AspNet.Mvc.Core +{ + public class CopyOnWriteDictionaryTest + { + [Fact] + public void ReadOperation_DelegatesToSourceDictionary_IfNoMutationsArePerformed() + { + // Arrange + var values = new List(); + var enumerator = Mock.Of>>(); + var sourceDictionary = new Mock>(MockBehavior.Strict); + sourceDictionary.SetupGet(d => d.Count) + .Returns(100) + .Verifiable(); + sourceDictionary.SetupGet(d => d.Values) + .Returns(values) + .Verifiable(); + sourceDictionary.Setup(d => d.ContainsKey("test-key")) + .Returns(value: true) + .Verifiable(); + sourceDictionary.Setup(d => d.GetEnumerator()) + .Returns(enumerator) + .Verifiable(); + sourceDictionary.Setup(d => d["key2"]) + .Returns("key2-value") + .Verifiable(); + object value; + sourceDictionary.Setup(d => d.TryGetValue("different-key", out value)) + .Returns(false) + .Verifiable(); + + var copyOnWriteDictionary = new CopyOnWriteDictionary(sourceDictionary.Object, + StringComparer.OrdinalIgnoreCase); + + // Act and Assert + Assert.Equal("key2-value", copyOnWriteDictionary["key2"]); + Assert.Equal(100, copyOnWriteDictionary.Count); + Assert.Same(values, copyOnWriteDictionary.Values); + Assert.True(copyOnWriteDictionary.ContainsKey("test-key")); + Assert.Same(enumerator, copyOnWriteDictionary.GetEnumerator()); + Assert.False(copyOnWriteDictionary.TryGetValue("different-key", out value)); + sourceDictionary.Verify(); + } + + [Fact] + public void ReadOperation_DoesNotDelegateToSourceDictionary_OnceAValueIsChanged() + { + // Arrange + var values = new List(); + var sourceDictionary = new Dictionary + { + { "key1", "value1" }, + { "key2", "value2" } + }; + var copyOnWriteDictionary = new CopyOnWriteDictionary(sourceDictionary, + StringComparer.OrdinalIgnoreCase); + + // Act + copyOnWriteDictionary["key2"] = "value3"; + + // Assert + Assert.Equal("value2", sourceDictionary["key2"]); + Assert.Equal(2, copyOnWriteDictionary.Count); + Assert.Equal("value1", copyOnWriteDictionary["key1"]); + Assert.Equal("value3", copyOnWriteDictionary["key2"]); + } + + [Fact] + public void ReadOperation_DoesNotDelegateToSourceDictionary_OnceDictionaryIsModified() + { + // Arrange + var values = new List(); + var sourceDictionary = new Dictionary + { + { "key1", "value1" }, + { "key2", "value2" } + }; + var copyOnWriteDictionary = new CopyOnWriteDictionary(sourceDictionary, + StringComparer.OrdinalIgnoreCase); + + // Act + copyOnWriteDictionary.Add("key3", "value3"); + copyOnWriteDictionary.Remove("key1"); + + + // Assert + Assert.Equal(2, sourceDictionary.Count); + Assert.Equal("value1", sourceDictionary["key1"]); + Assert.Equal(2, copyOnWriteDictionary.Count); + Assert.Equal("value2", copyOnWriteDictionary["KeY2"]); + Assert.Equal("value3", copyOnWriteDictionary["key3"]); + } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ViewDataDictionaryTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ViewDataDictionaryTest.cs new file mode 100644 index 0000000000..093a0a4105 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ViewDataDictionaryTest.cs @@ -0,0 +1,142 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.AspNet.Mvc.ModelBinding; +using Moq; +using Xunit; + +namespace Microsoft.AspNet.Mvc.Core +{ + public class ViewDataDictionaryTest + { + [Fact] + public void ConstructorWithOneParameterInitalizesMembers() + { + // Arrange + var metadataProvider = new EmptyModelMetadataProvider(); + + // Act + var viewData = new ViewDataDictionary(metadataProvider); + + // Assert + Assert.NotNull(viewData.ModelState); + Assert.NotNull(viewData.TemplateInfo); + Assert.Null(viewData.Model); + Assert.Null(viewData.ModelMetadata); + Assert.Equal(0, viewData.Count); + } + + [Fact] + public void ConstructorInitalizesMembers() + { + // Arrange + var metadataProvider = new EmptyModelMetadataProvider(); + var modelState = new ModelStateDictionary(); + + // Act + var viewData = new ViewDataDictionary(metadataProvider, modelState); + + // Assert + Assert.Same(modelState, viewData.ModelState); + Assert.NotNull(viewData.TemplateInfo); + Assert.Null(viewData.Model); + Assert.Null(viewData.ModelMetadata); + Assert.Equal(0, viewData.Count); + } + + [Fact] + public void SetModelUsesPassedInModelMetadataProvider() + { + // Arrange + var metadataProvider = new Mock(); + metadataProvider.Setup(m => m.GetMetadataForType(It.IsAny>(), typeof(TestModel))) + .Returns(new EmptyModelMetadataProvider().GetMetadataForType(null, typeof(TestModel))) + .Verifiable(); + var modelState = new ModelStateDictionary(); + var viewData = new TestViewDataDictionary(metadataProvider.Object, modelState); + var model = new TestModel(); + + // Act + viewData.SetModelPublic(model); + + // Assert + Assert.NotNull(viewData.ModelMetadata); + metadataProvider.Verify(); + } + + [Fact] + public void CopyConstructorInitalizesModelAndModelMetadataBasedOnSource() + { + // Arrange + var metadataProvider = new EmptyModelMetadataProvider(); + var model = new TestModel(); + var source = new ViewDataDictionary(metadataProvider) + { + Model = model + }; + source["foo"] = "bar"; + + // Act + var viewData = new ViewDataDictionary(source); + + // Assert + Assert.NotNull(viewData.ModelState); + Assert.NotNull(viewData.TemplateInfo); + Assert.NotSame(source.TemplateInfo, viewData.TemplateInfo); + Assert.Same(model, viewData.Model); + Assert.NotNull(viewData.ModelMetadata); + Assert.Equal(typeof(TestModel), viewData.ModelMetadata.ModelType); + Assert.Equal("bar", viewData["foo"]); + Assert.IsType>(viewData.Data); + } + + [Fact] + public void CopyConstructorUsesPassedInModel() + { + // Arrange + var metadataProvider = new EmptyModelMetadataProvider(); + var model = new TestModel(); + var source = new ViewDataDictionary(metadataProvider) + { + Model = "string model" + }; + source["key1"] = "value1"; + + // Act + var viewData = new ViewDataDictionary(source, model); + + // Assert + Assert.NotNull(viewData.ModelState); + Assert.NotNull(viewData.TemplateInfo); + Assert.Same(model, viewData.Model); + Assert.NotNull(viewData.ModelMetadata); + Assert.Equal(typeof(TestModel), viewData.ModelMetadata.ModelType); + Assert.Equal("value1", viewData["key1"]); + Assert.IsType>(viewData.Data); + } + + private class TestModel + { + } + + private class TestViewDataDictionary : ViewDataDictionary + { + public TestViewDataDictionary(IModelMetadataProvider modelMetadataProvider, + ModelStateDictionary modelState) + : base(modelMetadataProvider, modelState) + { + } + + public TestViewDataDictionary(ViewDataDictionary source) + : base(source) + { + } + + public void SetModelPublic(object value) + { + SetModel(value); + } + } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelStateDictionaryTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelStateDictionaryTest.cs index dcc0cdcba4..176759bff0 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelStateDictionaryTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelStateDictionaryTest.cs @@ -9,6 +9,28 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { public class ModelStateDictionaryTest { + [Fact] + public void CopyConstructor_CopiesModelStateData() + { + // Arrange + var modelState = new ModelState + { + Value = GetValueProviderResult("value") + }; + var source = new ModelStateDictionary + { + { "key", modelState } + }; + + // Act + var target = new ModelStateDictionary(source); + + // Assert + Assert.Equal(1, target.Count); + Assert.Same(modelState, target["key"]); + Assert.IsType>(target.InnerDictionary); + } + [Fact] public void AddModelErrorCreatesModelStateIfNotPresent() { @@ -162,15 +184,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { { "foo", new ModelState { - ValidationState = ModelValidationState.Valid, - Value = GetValueProviderResult("bar", "bar") + ValidationState = ModelValidationState.Valid, + Value = GetValueProviderResult("bar", "bar") } }, - { "baz", new ModelState - { - ValidationState = ModelValidationState.Valid, - Value = GetValueProviderResult("quux", "bar") - } + { "baz", new ModelState + { + ValidationState = ModelValidationState.Valid, + Value = GetValueProviderResult("quux", "bar") + } } };