PR comments

- change `AdditionalValues` type to `IDictionary<object, object>`
- copy `ModelMetadata.AdditionalValues` in `AssociatedMetadataProvider` sub-classes

nits:
- add XML comments
- correct indentation in `CachedModelMetadata`

src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/AssociatedMetadataProvider.cs

@@ -71,13 +71,38 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             return GetMetadataForParameterCore(modelAccessor, parameterName, parameter);
         }
 
-        // Override for creating the prototype metadata (without the accessor)
+        // Override for creating the prototype metadata (without the model accessor).
+        /// <summary>
+        /// Creates a new <typeparamref name="TModelMetadata"/> instance.
+        /// </summary>
+        /// <param name="attributes">The set of attributes relevant for the new instance.</param>
+        /// <param name="containerType">
+        /// <see cref="Type"/> containing this property. <c>null</c> unless this <typeparamref name="TModelMetadata"/>
+        /// describes a property.
+        /// </param>
+        /// <param name="modelType"><see cref="Type"/> this <typeparamref name="TModelMetadata"/> describes.</param>
+        /// <param name="propertyName">
+        /// Name of the property (in <paramref name="containerType"/>) or parameter this
+        /// <typeparamref name="TModelMetadata"/> describes. <c>null</c> or empty if this
+        /// <typeparamref name="TModelMetadata"/> describes a <see cref="Type"/>.
+        /// </param>
+        /// <returns>A new <typeparamref name="TModelMetadata"/> instance.</returns>
         protected abstract TModelMetadata CreateMetadataPrototype(IEnumerable<object> attributes,
                                                                   Type containerType,
                                                                   Type modelType,
                                                                   string propertyName);
 
-        // Override for applying the prototype + modelAccess to yield the final metadata
+        // Override for applying the prototype + model accessor to yield the final metadata.
+        /// <summary>
+        /// Creates a new <typeparamref name="TModelMetadata"/> instance based on a <paramref name="prototype"/>.
+        /// </summary>
+        /// <param name="prototype">
+        /// <typeparamref name="TModelMetadata"/> that provides the basis for new instance.
+        /// </param>
+        /// <param name="modelAccessor">Accessor for model value of new instance.</param>
+        /// <returns>
+        /// A new <typeparamref name="TModelMetadata"/> instance based on <paramref name="prototype"/>.
+        /// </returns>
         protected abstract TModelMetadata CreateMetadataFromPrototype(TModelMetadata prototype,
                                                                       Func<object> modelAccessor);
 

src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/CachedModelMetadata.cs

@@ -86,7 +86,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
         // Sealing for consistency with other properties.
         // ModelMetadata already caches the collection and we have no use case for a ComputeAdditionalValues() method.
         /// <inheritdoc />
-        public sealed override IDictionary<string, object> AdditionalValues
+        public sealed override IDictionary<object, object> AdditionalValues
         {
             get
             {
@@ -113,7 +113,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 _binderMetadata = value;
                 _isBinderMetadataComputed = true;
             }
-            }
+        }
 
         /// <inheritdoc />
         public sealed override string BinderModelName

src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataAnnotationsModelMetadataProvider.cs

@@ -6,8 +6,14 @@ using System.Collections.Generic;
 
 namespace Microsoft.AspNet.Mvc.ModelBinding
 {
+    /// <summary>
+    /// An <see cref="IModelMetadataProvider"/> implementation that provides
+    /// <see cref="CachedDataAnnotationsModelMetadata"/> instances. Those instances primarily calculate property values
+    /// using attributes from the <see cref="System.ComponentModel.DataAnnotations"/> namespace.
+    /// </summary>
     public class DataAnnotationsModelMetadataProvider : AssociatedMetadataProvider<CachedDataAnnotationsModelMetadata>
     {
+        /// <inheritdoc />
         protected override CachedDataAnnotationsModelMetadata CreateMetadataPrototype(
             IEnumerable<object> attributes,
             Type containerType,
@@ -17,11 +23,21 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             return new CachedDataAnnotationsModelMetadata(this, containerType, modelType, propertyName, attributes);
         }
 
+        /// <inheritdoc />
+        /// <remarks>
+        /// Copies only a few values from the <paramref name="prototype"/>. Unlikely the rest have been computed.
+        /// </remarks>
         protected override CachedDataAnnotationsModelMetadata CreateMetadataFromPrototype(
             CachedDataAnnotationsModelMetadata prototype,
             Func<object> modelAccessor)
         {
-            return new CachedDataAnnotationsModelMetadata(prototype, modelAccessor);
+            var metadata = new CachedDataAnnotationsModelMetadata(prototype, modelAccessor);
+            foreach (var keyValuePair in prototype.AdditionalValues)
+            {
+                metadata.AdditionalValues.Add(keyValuePair);
+            }
+
+            return metadata;
         }
     }
 }

src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/EmptyModelMetadataProvider.cs

@@ -6,24 +6,50 @@ using System.Collections.Generic;
 
 namespace Microsoft.AspNet.Mvc.ModelBinding
 {
+    /// <summary>
+    /// An <see cref="IModelMetadataProvider"/> that provides base <see cref="ModelMetadata"/> instances and does not
+    /// set most <see cref="ModelMetadata"/> properties. For example this provider does not use data annotations.
+    /// </summary>
+    /// <remarks>
+    /// Provided for efficiency in scenarios that require minimal <see cref="ModelMetadata"/> information.
+    /// </remarks>
     public class EmptyModelMetadataProvider : AssociatedMetadataProvider<ModelMetadata>
     {
+        /// <inheritdoc />
+        /// <remarks>Ignores <paramref name="attributes"/>.</remarks>
         protected override ModelMetadata CreateMetadataPrototype(IEnumerable<object> attributes,
                                                                  Type containerType,
-                                                                 Type modelType,
+                                                                 [NotNull] Type modelType,
                                                                  string propertyName)
         {
-            return new ModelMetadata(this, containerType, null, modelType, propertyName);
+            return new ModelMetadata(
+                this,
+                containerType,
+                modelAccessor: null,
+                modelType: modelType,
+                propertyName: propertyName);
         }
 
-        protected override ModelMetadata CreateMetadataFromPrototype(ModelMetadata prototype,
+        /// <inheritdoc />
+        /// <remarks>
+        /// Copies very few values from the <paramref name="prototype"/>. Likely <paramref name="prototype"/> has not
+        /// been modified except to add <see cref="ModelMetadata.AdditionalValues"/> entries.
+        /// </remarks>
+        protected override ModelMetadata CreateMetadataFromPrototype([NotNull] ModelMetadata prototype,
                                                                      Func<object> modelAccessor)
         {
-            return new ModelMetadata(this,
+            var metadata = new ModelMetadata(
-                                     prototype.ContainerType,
+                this,
-                                     modelAccessor,
+                prototype.ContainerType,
-                                     prototype.ModelType,
+                modelAccessor,
-                                     prototype.PropertyName);
+                prototype.ModelType,
+                prototype.PropertyName);
+            foreach (var keyValuePair in prototype.AdditionalValues)
+            {
+                metadata.AdditionalValues.Add(keyValuePair);
+            }
+
+            return metadata;
         }
     }
 }

src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/ModelMetadata.cs

@@ -50,8 +50,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
         /// <summary>
         /// Gets a collection of additional information about the model.
         /// </summary>
-        public virtual IDictionary<string, object> AdditionalValues { get; }
+        public virtual IDictionary<object, object> AdditionalValues { get; } = new Dictionary<object, object>();
-            = new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase);
 
         /// <summary>
         /// Gets or sets the name of a model if specified explicitly using <see cref="IModelNameProvider"/>.

test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/ModelMetadataTest.cs

@@ -150,7 +150,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 modelAccessor: () => null,
                 modelType: typeof(object),
                 propertyName: null);
-            var valuesDictionary = new Dictionary<string, object>
+            var valuesDictionary = new Dictionary<object, object>
             {
                 { "key1", new object() },
                 { "key2", "value2" },

test/WebSites/ModelBindingWebSite/AdditionalValuesMetadataProvider.cs

@@ -15,19 +15,6 @@ namespace ModelBindingWebSite
         public static readonly string GroupNameKey = "__GroupName";
         private static Guid _guid = new Guid("7d6d0de2-8d59-49ac-99cc-881423b75a76");
 
-        protected override CachedDataAnnotationsModelMetadata CreateMetadataFromPrototype(
-            CachedDataAnnotationsModelMetadata prototype,
-            Func<object> modelAccessor)
-        {
-            var metadata = base.CreateMetadataFromPrototype(prototype, modelAccessor);
-            foreach (var keyValuePair in prototype.AdditionalValues)
-            {
-                metadata.AdditionalValues.Add(keyValuePair);
-            }
-
-            return metadata;
-        }
-
         protected override CachedDataAnnotationsModelMetadata CreateMetadataPrototype(
             IEnumerable<object> attributes,
             Type containerType,

test/WebSites/ModelBindingWebSite/Controllers/ModelMetadataController.cs

@@ -12,7 +12,7 @@ namespace ModelBindingWebSite.Controllers
     public class ModelMetadataController
     {
         [HttpGet(template: "/AdditionalValues")]
-        public IDictionary<string, object> GetAdditionalValues([FromServices] IModelMetadataProvider provider)
+        public IDictionary<object, object> GetAdditionalValues([FromServices] IModelMetadataProvider provider)
         {
             var metadata = provider.GetMetadataForType(
                 modelAccessor: null,

test/WebSites/ModelBindingWebSite/Startup.cs

@@ -18,8 +18,8 @@ namespace ModelBindingWebSite
             // Set up application services
             app.UseServices(services =>
             {
-                // Override the IModelMetadataProvider AddMvc would normally use. Make service a singleton though
+                // Override the IModelMetadataProvider AddMvc would normally add.
-                // AddMvc would configure DataAnnotationsModelMetadataProvider as transient.
+                // ModelMetadataController relies on additional values AdditionalValuesMetadataProvider provides.
                 services.AddSingleton<IModelMetadataProvider, AdditionalValuesMetadataProvider>();
 
                 // Add MVC services to the services container