[Fixes #2259] Use fast property setter in MutableObjectModelBinder

src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs

@@ -480,7 +480,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             {
                 try
                 {
-                    property.SetValue(bindingContext.Model, value);
+                    propertyMetadata.PropertySetter(bindingContext.Model, value);
                 }
                 catch (Exception ex)
                 {

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

@@ -51,9 +51,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
         public ModelMetadata[] Properties { get; set; }
 
         /// <summary>
-        /// Gets or sets a property accessor delegate to get the property value from a model object.
+        /// Gets or sets a property getter delegate to get the property value from a model object.
         /// </summary>
-        public Func<object, object> PropertyAccessor { get; set; }
+        public Func<object, object> PropertyGetter { get; set; }
 
         /// <summary>
         /// Gets or sets a property setter delegate to set the property value on a model object.

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

@@ -284,9 +284,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
                     {
                         _isReadOnly = BindingMetadata.IsReadOnly;
                     }
+                    else if (MetadataKind == ModelMetadataKind.Type)
+                    {
+                        _isReadOnly = false;
+                    }
                     else
                     {
-                        _isReadOnly = _details.PropertySetter != null;
+                        _isReadOnly = _details.PropertySetter == null;
                     }
                 }
 
@@ -407,5 +411,23 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
                 return _validatorMetadata;
             }
         }
+
+        /// <inheritdoc />
+        public override Func<object, object> PropertyGetter
+        {
+            get
+            {
+                return _details.PropertyGetter;
+            }
+        }
+
+        /// <inheritdoc />
+        public override Action<object, object> PropertySetter
+        {
+            get
+            {
+                return _details.PropertySetter;
+            }
+        }
     }
 }

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

@@ -4,7 +4,6 @@
 using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
-using System.Reflection;
 using Microsoft.Framework.Internal;
 
 namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
@@ -120,12 +119,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
                     propertyHelper.Property));
 
                 var propertyEntry = new DefaultMetadataDetails(propertyKey, attributes);
-                if (propertyHelper.Property.CanRead && propertyHelper.Property.GetMethod?.IsPrivate == true)
+                if (propertyHelper.Property.CanRead && propertyHelper.Property.GetMethod?.IsPublic == true)
                 {
-                    propertyEntry.PropertyAccessor = PropertyHelper.MakeFastPropertyGetter(propertyHelper.Property);
+                    propertyEntry.PropertyGetter = PropertyHelper.MakeFastPropertyGetter(propertyHelper.Property);
                 }
 
-                if (propertyHelper.Property.CanWrite && propertyHelper.Property.SetMethod?.IsPrivate == true)
+                if (propertyHelper.Property.CanWrite &&
+                    propertyHelper.Property.SetMethod?.IsPublic == true &&
+                    !key.ModelType.IsValueType())
                 {
                     propertyEntry.PropertySetter = PropertyHelper.MakeFastPropertySetter(propertyHelper.Property);
                 }

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

@@ -298,5 +298,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
         {
             return DisplayName ?? PropertyName ?? ModelType.Name;
         }
+
+        /// <summary>
+        /// Gets or sets a property getter delegate to get the property value from a model object.
+        /// </summary>
+        public abstract Func<object, object> PropertyGetter { get; }
+
+        /// <summary>
+        /// Gets or sets a property setter delegate to set the property value on a model object.
+        /// </summary>
+        public abstract Action<object, object> PropertySetter { get; }
     }
 } 

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

@@ -411,8 +411,93 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
             Assert.Equal(firstPropertiesEvaluation, secondPropertiesEvaluation);
         }
 
+        [Fact]
+        public void IsReadOnly_ReturnsFalse_ForType()
+        {
+            // Arrange
+            var detailsProvider = new EmptyCompositeMetadataDetailsProvider();
+            var provider = new DefaultModelMetadataProvider(detailsProvider);
+
+            var key = ModelMetadataIdentity.ForType(typeof(int[]));
+            var cache = new DefaultMetadataDetails(key, new object[0]);
+
+            var metadata = new DefaultModelMetadata(provider, detailsProvider, cache);
+
+            // Act
+            var isReadOnly = metadata.IsReadOnly;
+
+            // Assert
+            Assert.False(isReadOnly);
+        }
+
+        [Fact]
+        public void IsReadOnly_ReturnsTrue_ForPrivateSetProperty()
+        {
+            // Arrange
+            var detailsProvider = new EmptyCompositeMetadataDetailsProvider();
+            var provider = new DefaultModelMetadataProvider(detailsProvider);
+
+            var key = ModelMetadataIdentity.ForType(typeof(TypeWithProperties));
+            var cache = new DefaultMetadataDetails(key, new object[0]);
+
+            var metadata = new DefaultModelMetadata(provider, detailsProvider, cache);
+
+            // Act
+            var isReadOnly = metadata.Properties["PublicGetPrivateSetProperty"].IsReadOnly;
+
+            // Assert
+            Assert.True(isReadOnly);
+        }
+
+        [Fact]
+        public void IsReadOnly_ReturnsTrue_ForProtectedSetProperty()
+        {
+            // Arrange
+            var detailsProvider = new EmptyCompositeMetadataDetailsProvider();
+            var provider = new DefaultModelMetadataProvider(detailsProvider);
+
+            var key = ModelMetadataIdentity.ForType(typeof(TypeWithProperties));
+            var cache = new DefaultMetadataDetails(key, new object[0]);
+
+            var metadata = new DefaultModelMetadata(provider, detailsProvider, cache);
+
+            // Act
+            var isReadOnly = metadata.Properties["PublicGetProtectedSetProperty"].IsReadOnly;
+
+            // Assert
+            Assert.True(isReadOnly);
+        }
+
+        [Fact]
+        public void IsReadOnly_ReturnsFalse_ForPublicSetProperty()
+        {
+            // Arrange
+            var detailsProvider = new EmptyCompositeMetadataDetailsProvider();
+            var provider = new DefaultModelMetadataProvider(detailsProvider);
+
+            var key = ModelMetadataIdentity.ForType(typeof(TypeWithProperties));
+            var cache = new DefaultMetadataDetails(key, new object[0]);
+
+            var metadata = new DefaultModelMetadata(provider, detailsProvider, cache);
+
+            // Act
+            var isReadOnly = metadata.Properties["PublicGetPublicSetProperty"].IsReadOnly;
+
+            // Assert
+            Assert.False(isReadOnly);
+        }
+
         private void ActionMethod(string input)
         {
         }
+
+        private class TypeWithProperties
+        {
+            public string PublicGetPrivateSetProperty { get; private set; }
+
+            public int PublicGetProtectedSetProperty { get; protected set; }
+
+            public int PublicGetPublicSetProperty { get; set; }
+        }
     }
 }

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

@@ -696,11 +696,71 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
             Assert.False(metadata.IsRequired);
         }
 
+        [Fact]
+        public void PropertySetter_NotNull_ForPublicSetProperty()
+        {
+            // Arrange
+            var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
+
+            // Act
+            var metadata = metadataProvider.GetMetadataForProperty(
+                typeof(ClassWithPublicSetProperty),
+                nameof(ClassWithPublicSetProperty.StringProperty));
+
+            // Assert
+            Assert.NotNull(metadata.PropertySetter);
+            Assert.NotNull(metadata.PropertyGetter);
+        }
+
+        [Fact]
+        public void PropertySetter_Null_ForPrivateSetProperty()
+        {
+            // Arrange
+            var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
+
+            // Act
+            var metadata = metadataProvider.GetMetadataForProperty(
+                typeof(ClassWithPrivateSetProperty),
+                nameof(ClassWithPrivateSetProperty.StringProperty));
+
+            // Assert
+            Assert.Null(metadata.PropertySetter);
+            Assert.NotNull(metadata.PropertyGetter);
+        }
+
+        [Fact]
+        public void Metadata_Null_ForPrivateGetProperty()
+        {
+            // Arrange
+            var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
+            var type = typeof(ClassWithPrivateGetProperty);
+            var propertyName = nameof(ClassWithPrivateGetProperty.StringProperty);
+
+            // Act & Assert
+            var metadata = metadataProvider.GetMetadataForType(type);
+            Assert.Null(metadata.Properties[propertyName]);
+        }
+
         private IModelMetadataProvider CreateProvider(params object[] attributes)
         {
             return new AttributeInjectModelMetadataProvider(attributes);
         }
 
+        private class ClassWithPrivateSetProperty
+        {
+            public string StringProperty { private set; get; }
+        }
+
+        private class ClassWithPublicSetProperty
+        {
+            public string StringProperty { set; get; }
+        }
+
+        private class ClassWithPrivateGetProperty
+        {
+            public string StringProperty { set; private get; }
+        }
+
         [DataContract]
         private class ClassWithDataMemberIsRequiredTrue
         {

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

@@ -412,6 +412,22 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                     throw new NotImplementedException();
                 }
             }
+
+            public override Func<object, object> PropertyGetter
+            {
+                get
+                {
+                    throw new NotImplementedException();
+                }
+            }
+
+            public override Action<object, object> PropertySetter
+            {
+                get
+                {
+                    throw new NotImplementedException();
+                }
+            }
         }
     }
 }