Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Commit

Permalink
Remove [BindProperty] on class
Browse files Browse the repository at this point in the history 
This isn't a good fit with consistency with controllers. Discussed with
@DamianEdwards and we agreed to remove this for now and bring it back in
the future if there's a real need for it.
  • Loading branch information
@rynowak
rynowak committed Jun 28, 2017
1 parent 12ea28a commit 1886d53
Show file tree
Hide file tree
Showing 4 changed files with 4 additions and 115 deletions.
2 changes: 1 addition & 1 deletion src/Microsoft.AspNetCore.Mvc.Core/BindPropertyAttribute.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

namespace Microsoft.AspNetCore.Mvc
{
[AttributeUsage(AttributeTargets.Property | AttributeTargets.Class, AllowMultiple = false, Inherited = true)]
[AttributeUsage(AttributeTargets.Property, AllowMultiple = false, Inherited = true)]
public class BindPropertyAttribute : Attribute, IModelNameProvider, IBinderTypeProviderMetadata, IRequestPredicateProvider
{
private static readonly Func<ActionContext, bool> _supportsAllRequests = (c) => true;
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PagesBaseClassAttribute.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure
{
/// <summary>
/// An attribute for base classes for Pages and PageModels. Applying this attribute to a type
/// suppresses discovery of handler methods and bound properties for that type.
/// suppresses discovery of handler methods for that type.
/// </summary>
[AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = false)]
public class PagesBaseClassAttribute : Attribute
Expand Down
19 changes: 2 additions & 17 deletions src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageLoader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,34 +239,19 @@ internal static PageBoundPropertyDescriptor[] CreateBoundProperties(TypeInfo typ
{
var properties = PropertyHelper.GetVisibleProperties(type.AsType());

// If the type has a [BindPropertyAttribute] then we'll consider any and all public properties bindable.
var bindPropertyOnType = type.GetCustomAttribute<BindPropertyAttribute>();

var results = new List<PageBoundPropertyDescriptor>();
for (var i = 0; i < properties.Length; i++)
{
var property = properties[i];
var bindingInfo = BindingInfo.GetBindingInfo(property.Property.GetCustomAttributes());

// If there's no binding info then that means there are no model binding attributes on the
// property. So we won't bind this property unless there's a [BindProperty] on the type.
if (bindingInfo == null && bindPropertyOnType == null)
// property. So we won't bind this property.
if (bindingInfo == null)
{
continue;
}

// If this property is declared as part of a pages base class, then it's likely infrastructure,
// so skip it.
if (property.Property.DeclaringType.GetTypeInfo().IsDefined(typeof(PagesBaseClassAttribute)))
{
continue;
}

bindingInfo = bindingInfo ?? new BindingInfo();

// Allow a predicate on the class to cascade if it wasn't set on the property.
bindingInfo.RequestPredicate = bindingInfo.RequestPredicate ?? ((IRequestPredicateProvider)bindPropertyOnType)?.RequestPredicate;

var descriptor = new PageBoundPropertyDescriptor()
{
BindingInfo = bindingInfo,
Expand Down
96 changes: 0 additions & 96 deletions test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/DefaultPageLoaderTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -537,31 +537,6 @@ private class ModelWithBindProperty
public int IgnoreMe { get; set; }
}

// Additionally [BindProperty] on a property can opt-in a property
[Fact]
public void CreateBoundProperties_BindPropertyAttributeOnModel_OptsInAllProperties()
{
// Arrange
var type = typeof(ModelWithBindPropertyOnClass).GetTypeInfo();

// Act
var results = DefaultPageLoader.CreateBoundProperties(type);

// Assert
Assert.Collection(
results.OrderBy(p => p.Property.Name),
p =>
{
Assert.Equal("Property", p.Property.Name);
});
}

[BindProperty]
private class ModelWithBindPropertyOnClass : EmptyPageModel
{
public int Property { get; set; }
}

[Fact]
public void CreateBoundProperties_SupportsGet_OnProperty()
{
Expand Down Expand Up @@ -599,77 +574,6 @@ private class ModelSupportsGetOnProperty
public int IgnoreMe { get; set; }
}

[Fact]
public void CreateBoundProperties_SupportsGet_OnClass()
{
// Arrange
var type = typeof(ModelSupportsGetOnClass).GetTypeInfo();

// Act
var results = DefaultPageLoader.CreateBoundProperties(type);

// Assert
Assert.Collection(
results.OrderBy(p => p.Property.Name),
p =>
{
Assert.Equal("Property", p.Property.Name);
Assert.NotNull(p.BindingInfo.RequestPredicate);
Assert.True(p.BindingInfo.RequestPredicate(new ActionContext()
{
HttpContext = new DefaultHttpContext()
{
Request =
{
Method ="GET",
}
}
}));
});
}

[BindProperty(SupportsGet = true)]
private class ModelSupportsGetOnClass : EmptyPageModel
{
public int Property { get; set; }
}

[Fact]
public void CreateBoundProperties_SupportsGet_Override()
{
// Arrange
var type = typeof(ModelSupportsGetOverride).GetTypeInfo();

// Act
var results = DefaultPageLoader.CreateBoundProperties(type);

// Assert
Assert.Collection(
results.OrderBy(p => p.Property.Name),
p =>
{
Assert.Equal("Property", p.Property.Name);
Assert.NotNull(p.BindingInfo.RequestPredicate);
Assert.False(p.BindingInfo.RequestPredicate(new ActionContext()
{
HttpContext = new DefaultHttpContext()
{
Request =
{
Method ="GET",
}
}
}));
});
}

[BindProperty(SupportsGet = true)]
private class ModelSupportsGetOverride : EmptyPageModel
{
[BindProperty(SupportsGet = false)]
public int Property { get; set; }
}

[Theory]
[InlineData("Foo")]
[InlineData("On")]
Expand Down

0 comments on commit 1886d53

Please sign in to comment.