diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs index 184a1e9805..f589c33eb4 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs +++ b/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) { diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DefaultMetadataDetails.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DefaultMetadataDetails.cs index f0766ed1bc..27ea450313 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DefaultMetadataDetails.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DefaultMetadataDetails.cs @@ -51,9 +51,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata public ModelMetadata[] Properties { get; set; } /// - /// 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. /// - public Func PropertyAccessor { get; set; } + public Func PropertyGetter { get; set; } /// /// Gets or sets a property setter delegate to set the property value on a model object. diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DefaultModelMetadata.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DefaultModelMetadata.cs index 1195957bad..f995540ccf 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DefaultModelMetadata.cs +++ b/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; } } + + /// + public override Func PropertyGetter + { + get + { + return _details.PropertyGetter; + } + } + + /// + public override Action PropertySetter + { + get + { + return _details.PropertySetter; + } + } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DefaultModelMetadataProvider.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DefaultModelMetadataProvider.cs index 8ec2c57934..a8bd6b021d 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DefaultModelMetadataProvider.cs +++ b/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); } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/ModelMetadata.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelMetadata.cs index d19212f94a..a23fc98ab2 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/ModelMetadata.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelMetadata.cs @@ -298,5 +298,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { return DisplayName ?? PropertyName ?? ModelType.Name; } + + /// + /// Gets or sets a property getter delegate to get the property value from a model object. + /// + public abstract Func PropertyGetter { get; } + + /// + /// Gets or sets a property setter delegate to set the property value on a model object. + /// + public abstract Action PropertySetter { get; } } } diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DefaultModelMetadataTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DefaultModelMetadataTest.cs index 3f05f2224b..e5273826a9 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DefaultModelMetadataTest.cs +++ b/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; } + } } } diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/ModelMetadataProviderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/ModelMetadataProviderTest.cs index ca86e01f86..84f3b60158 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/ModelMetadataProviderTest.cs +++ b/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 { diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/ModelMetadataTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/ModelMetadataTest.cs index 774e4b5b0e..2ec98eea4b 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/ModelMetadataTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/ModelMetadataTest.cs @@ -412,6 +412,22 @@ namespace Microsoft.AspNet.Mvc.ModelBinding throw new NotImplementedException(); } } + + public override Func PropertyGetter + { + get + { + throw new NotImplementedException(); + } + } + + public override Action PropertySetter + { + get + { + throw new NotImplementedException(); + } + } } } }