Modifying ViewDataDictionary and RouteValueDictionary to copy on write

instead of eagerly copying.

Partial fix for #878

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
+{
+    /// <summary>
+    /// A <see cref="IDictionary{TKey, TValue}"/> that defers creating a shallow copy of the source dictionary until
+    ///  a mutative operation has been performed on it.
+    /// </summary>
+    internal class CopyOnWriteDictionary<TKey, TValue> : IDictionary<TKey, TValue>
+    {
+        private readonly IDictionary<TKey, TValue> _sourceDictionary;
+        private readonly IEqualityComparer<TKey> _comparer;
+        private IDictionary<TKey, TValue> _innerDictionary;
+
+        public CopyOnWriteDictionary([NotNull] IDictionary<TKey, TValue> sourceDictionary,
+                                     [NotNull] IEqualityComparer<TKey> comparer)
+        {
+            _sourceDictionary = sourceDictionary;
+            _comparer = comparer;
+        }
+
+        private IDictionary<TKey, TValue> ReadDictionary
+        {
+            get
+            {
+                return _innerDictionary ?? _sourceDictionary;
+            }
+        }
+
+        private IDictionary<TKey, TValue> WriteDictionary
+        {
+            get
+            {
+                if (_innerDictionary == null)
+                {
+                    _innerDictionary = new Dictionary<TKey, TValue>(_sourceDictionary,
+                                                                    _comparer);
+                }
+
+                return _innerDictionary;
+            }
+        }
+
+        public virtual ICollection<TKey> Keys
+        {
+            get
+            {
+                return ReadDictionary.Keys;
+            }
+        }
+
+        public virtual ICollection<TValue> 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<TKey, TValue> item)
+        {
+            WriteDictionary.Add(item);
+        }
+
+        public virtual void Clear()
+        {
+            WriteDictionary.Clear();
+        }
+
+        public virtual bool Contains(KeyValuePair<TKey, TValue> item)
+        {
+            return ReadDictionary.Contains(item);
+        }
+
+        public virtual void CopyTo([NotNull] KeyValuePair<TKey, TValue>[] array, int arrayIndex)
+        {
+            ReadDictionary.CopyTo(array, arrayIndex);
+        }
+
+        public bool Remove(KeyValuePair<TKey, TValue> item)
+        {
+            return WriteDictionary.Remove(item);
+        }
+
+        public virtual IEnumerator<KeyValuePair<TKey, TValue>> GetEnumerator()
+        {
+            return ReadDictionary.GetEnumerator();
+        }
+
+        IEnumerator IEnumerable.GetEnumerator()
+        {
+            return GetEnumerator();
+        }
+    }
+}

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<string, object>(StringComparer.OrdinalIgnoreCase),
+                   templateInfo: new TemplateInfo())
         {
-            ModelState = modelState;
-            TemplateInfo = new TemplateInfo();
-            _data = new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase);
-            _metadataProvider = metadataProvider;
         }
 
         /// <summary>
@@ -45,24 +45,26 @@ namespace Microsoft.AspNet.Mvc
         /// exceptions a derived class may throw when <see cref="SetModel"/> is called.
         /// </summary>
         public ViewDataDictionary([NotNull] ViewDataDictionary source, object model)
-            : this(source.MetadataProvider)
+            : this(source.MetadataProvider,
+                   new ModelStateDictionary(source.ModelState),
+                   new CopyOnWriteDictionary<string, object>(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<string, object> 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<string, object> Data
+        {
+            get { return _data; }
+        }
+
         public object Eval(string expression)
         {
             var info = GetViewDataInfo(expression);

src/Microsoft.AspNet.Mvc.ModelBinding/ModelStateDictionary.cs

@@ -11,19 +11,17 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
 {
     public class ModelStateDictionary : IDictionary<string, ModelState>
     {
-        private readonly IDictionary<string, ModelState> _innerDictionary =
+        private readonly IDictionary<string, ModelState> _innerDictionary;
-            new Dictionary<string, ModelState>(StringComparer.OrdinalIgnoreCase);
 
         public ModelStateDictionary()
         {
+            _innerDictionary = new Dictionary<string, ModelState>(StringComparer.OrdinalIgnoreCase);
         }
 
         public ModelStateDictionary([NotNull] ModelStateDictionary dictionary)
         {
-            foreach (var entry in dictionary)
+            _innerDictionary = new CopyOnWriteDictionary<string, ModelState>(dictionary,
-            {
+                                                                             StringComparer.OrdinalIgnoreCase);
-                _innerDictionary.Add(entry.Key, entry.Value);
-            }
         }
 
         #region IDictionary properties
@@ -76,6 +74,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             }
         }
 
+        // For unit testing
+        internal IDictionary<string, ModelState> InnerDictionary
+        {
+            get { return _innerDictionary; }
+        }
+
         public void AddModelError([NotNull] string key, [NotNull] Exception exception)
         {
             var modelState = GetModelStateForKey(key);

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<object>();
+            var enumerator = Mock.Of<IEnumerator<KeyValuePair<string, object>>>();
+            var sourceDictionary = new Mock<IDictionary<string, object>>(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<string, object>(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<object>();
+            var sourceDictionary = new Dictionary<string, object>
+            {
+                { "key1", "value1" },
+                { "key2", "value2" }
+            };
+            var copyOnWriteDictionary = new CopyOnWriteDictionary<string, object>(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<object>();
+            var sourceDictionary = new Dictionary<string, object>
+            {
+                { "key1", "value1" },
+                { "key2", "value2" }
+            };
+            var copyOnWriteDictionary = new CopyOnWriteDictionary<string, object>(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"]);
+        }
+    }
+}

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<IModelMetadataProvider>();
+            metadataProvider.Setup(m => m.GetMetadataForType(It.IsAny<Func<object>>(), 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<CopyOnWriteDictionary<string, object>>(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<CopyOnWriteDictionary<string, object>>(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);
+            }
+        }
+    }
+}

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<CopyOnWriteDictionary<string, ModelState>>(target.InnerDictionary);
+        }
+
         [Fact]
         public void AddModelErrorCreatesModelStateIfNotPresent()
         {
@@ -162,15 +184,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             {
                 { "foo", new ModelState
                         {
-                            ValidationState = ModelValidationState.Valid, 
+                            ValidationState = ModelValidationState.Valid,
-                            Value = GetValueProviderResult("bar", "bar") 
+                            Value = GetValueProviderResult("bar", "bar")
                         }
                 },
-                { "baz", new ModelState 
+                { "baz", new ModelState
-                         { 
+                         {
-                             ValidationState = ModelValidationState.Valid, 
+                             ValidationState = ModelValidationState.Valid,
-                             Value = GetValueProviderResult("quux", "bar") 
+                             Value = GetValueProviderResult("quux", "bar")
-                         } 
+                         }
                 }
             };