Making Pages Binding Consistent

This changeset reckonciles the binding work we did for pages with
controllers.

A quick summary:
- Moves [BindProperty] to the MVC namespace (#6401)
- Makes [FromRoute] and friends behave consistently (#6402)
- Makes [BindProperty] work with controllers (untracked)

src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/BindingInfo.cs

@@ -34,6 +34,7 @@ public BindingInfo(BindingInfo other)
             BinderModelName = other.BinderModelName;
             BinderType = other.BinderType;
             PropertyFilterProvider = other.PropertyFilterProvider;
+            RequestPredicate = other.RequestPredicate;
         }
 
         /// <summary>
@@ -56,6 +57,12 @@ public BindingInfo(BindingInfo other)
         /// </summary>
         public IPropertyFilterProvider PropertyFilterProvider { get; set; }
 
+        /// <summary>
+        /// Gets or sets a predicate which determines whether or not the model should be bound based on state
+        /// from the current request.
+        /// </summary>
+        public Func<ActionContext, bool> RequestPredicate { get; set; }
+
         /// <summary>
         /// Constructs a new instance of <see cref="BindingInfo"/> from the given <paramref name="attributes"/>.
         /// </summary>
@@ -113,6 +120,13 @@ public static BindingInfo GetBindingInfo(IEnumerable<object> attributes)
                 bindingInfo.PropertyFilterProvider = new CompositePropertyFilterProvider(propertyFilterProviders);
             }
 
+            // RequestPredicate
+            foreach (var requestPredicateProvider in attributes.OfType<IRequestPredicateProvider>())
+            {
+                isBindingInfoPresent = true;
+                bindingInfo.RequestPredicate = requestPredicateProvider.RequestPredicate;
+            }
+
             return isBindingInfoPresent ? bindingInfo : null;
         }
 

src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/IRequestPredicateProvider.cs

@@ -0,0 +1,20 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+
+namespace Microsoft.AspNetCore.Mvc.ModelBinding
+{
+    /// <summary>
+    /// An interface that allows a top-level model to be bound or not bound based on state assocated
+    /// with the current request.
+    /// </summary>
+    public interface IRequestPredicateProvider
+    {
+        /// <summary>
+        /// Gets a function which determines whether or not the model object should be bound based
+        /// on the current request.
+        /// </summary>
+        Func<ActionContext, bool> RequestPredicate { get; }
+    }
+}

src/Microsoft.AspNetCore.Mvc.RazorPages/BindPropertyAttribute.cs → src/Microsoft.AspNetCore.Mvc.Core/BindPropertyAttribute.cs

@@ -4,10 +4,10 @@
 using System;
 using Microsoft.AspNetCore.Mvc.ModelBinding;
 
-namespace Microsoft.AspNetCore.Mvc.RazorPages
+namespace Microsoft.AspNetCore.Mvc
 {
     [AttributeUsage(AttributeTargets.Property | AttributeTargets.Class, AllowMultiple = false, Inherited = true)]
-    public class BindPropertyAttribute : Attribute, IModelNameProvider, IBinderTypeProviderMetadata
+    public class BindPropertyAttribute : Attribute, IModelNameProvider, IBinderTypeProviderMetadata, IRequestPredicateProvider
     {
         private BindingSource _bindingSource;
 
@@ -35,5 +35,18 @@ protected set
 
         /// <inheritdoc />
         public string Name { get; set; }
+
+        Func<ActionContext, bool> IRequestPredicateProvider.RequestPredicate
+        {
+            get
+            {
+                return SupportsGet ? (ActionContext c) => true : (Func<ActionContext, bool>)IsNonGetRequest;
+            }
+        }
+
+        private static bool IsNonGetRequest(ActionContext context)
+        {
+            return !string.Equals(context.HttpContext.Request.Method, "GET", StringComparison.OrdinalIgnoreCase);
+        }
     }
 }

src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ParameterBinder.cs

@@ -152,6 +152,11 @@ public class ParameterBinder
                 throw new ArgumentNullException(nameof(metadata));
             }
 
+            if (parameter.BindingInfo?.RequestPredicate?.Invoke(actionContext) == false)
+            {
+                return ModelBindingResult.Failed();
+            }
+
             var modelBindingContext = DefaultModelBindingContext.CreateBindingContext(
                 actionContext,
                 valueProvider,

src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageBoundPropertyDescriptor.cs

@@ -9,7 +9,5 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure
     public class PageBoundPropertyDescriptor : ParameterDescriptor
     {
         public PropertyInfo Property { get; set; }
-
-        public bool SupportsGet { get; set; }
     }
 }

src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageLoader.cs

@@ -248,26 +248,31 @@ internal static PageBoundPropertyDescriptor[] CreateBoundProperties(TypeInfo typ
                 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)
                 {
                     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;
                 }
 
-                var bindPropertyOnProperty = property.Property.GetCustomAttribute<BindPropertyAttribute>();
-                var supportsGet = bindPropertyOnProperty?.SupportsGet ?? bindPropertyOnType?.SupportsGet ?? false;
+                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 ?? new BindingInfo(),
+                    BindingInfo = bindingInfo,
                     Name = property.Name,
                     Property = property.Property,
                     ParameterType = property.Property.PropertyType,
-                    SupportsGet = supportsGet,
                 };
 
                 results.Add(descriptor);

src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PagePropertyBinderFactory.cs

@@ -7,7 +7,6 @@
 using Microsoft.AspNetCore.Mvc.Abstractions;
 using Microsoft.AspNetCore.Mvc.Internal;
 using Microsoft.AspNetCore.Mvc.ModelBinding;
-using Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure;
 
 namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
 {
@@ -56,16 +55,9 @@ Task Bind(PageContext pageContext, object instance)
             IList<ParameterDescriptor> properties,
             IList<ModelMetadata> metadata)
         {
-            var isGet = string.Equals("GET", pageContext.HttpContext.Request.Method, StringComparison.OrdinalIgnoreCase);
-
             var valueProvider = await CompositeValueProvider.CreateAsync(pageContext, pageContext.ValueProviderFactories);
             for (var i = 0; i < properties.Count; i++)
             {
-                if (isGet && !((PageBoundPropertyDescriptor)properties[i]).SupportsGet)
-                {
-                    continue;
-                }
-
                 var result = await parameterBinder.BindModelAsync(pageContext, valueProvider, properties[i]);
                 if (result.IsModelSet)
                 {

test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerBinderDelegateProviderTest.cs

@@ -13,6 +13,7 @@
 using Microsoft.AspNetCore.Mvc.ModelBinding;
 using Microsoft.AspNetCore.Mvc.ModelBinding.Binders;
 using Microsoft.AspNetCore.Mvc.ModelBinding.Validation;
+using Microsoft.AspNetCore.Mvc.RazorPages;
 using Microsoft.AspNetCore.Routing;
 using Moq;
 using Xunit;
@@ -490,6 +491,125 @@ public async Task BindActionArgumentsAsync_SetsNullValues_ForNullableProperties(
             Assert.Null(controller.NullableProperty);
         }
 
+        [Fact]
+        public async Task BindActionArgumentsAsync_SupportsRequestPredicate_ForPropertiesAndParameters_NotBound()
+        {
+            // Arrange
+            var actionDescriptor = GetActionDescriptor();
+
+            actionDescriptor.Parameters.Add(new ParameterDescriptor
+            {
+                Name = "test-parameter",
+                BindingInfo = new BindingInfo()
+                {
+                    BindingSource = BindingSource.Custom,
+
+                    // Simulates [BindProperty] on a parameter
+                    RequestPredicate = ((IRequestPredicateProvider)new BindPropertyAttribute()).RequestPredicate,
+                },
+                ParameterType = typeof(string)
+            });
+
+            actionDescriptor.BoundProperties.Add(new ParameterDescriptor
+            {
+                Name = nameof(TestController.NullableProperty),
+                BindingInfo = new BindingInfo()
+                {
+                    BindingSource = BindingSource.Custom,
+
+                    // Simulates [BindProperty] on a property
+                    RequestPredicate = ((IRequestPredicateProvider)new BindPropertyAttribute()).RequestPredicate,
+                },
+                ParameterType = typeof(string)
+            });
+
+            var controllerContext = GetControllerContext(actionDescriptor);
+            controllerContext.HttpContext.Request.Method = "GET";
+
+            var controller = new TestController();
+            var arguments = new Dictionary<string, object>(StringComparer.Ordinal);
+
+            var binder = new StubModelBinder(ModelBindingResult.Success(model: null));
+            var factory = GetModelBinderFactory(binder);
+            var parameterBinder = GetParameterBinder(factory);
+
+            // Some non default value.
+            controller.NullableProperty = -1;
+
+            // Act
+            var binderDelegate = ControllerBinderDelegateProvider.CreateBinderDelegate(
+                parameterBinder,
+                factory,
+                TestModelMetadataProvider.CreateDefaultProvider(),
+                actionDescriptor);
+
+            await binderDelegate(controllerContext, controller, arguments);
+
+            // Assert
+            Assert.Equal(-1, controller.NullableProperty);
+            Assert.DoesNotContain("test-parameter", arguments.Keys);
+        }
+
+        [Fact]
+        public async Task BindActionArgumentsAsync_SupportsRequestPredicate_ForPropertiesAndParameters_Bound()
+        {
+            // Arrange
+            var actionDescriptor = GetActionDescriptor();
+
+            actionDescriptor.Parameters.Add(new ParameterDescriptor
+            {
+                Name = "test-parameter",
+                BindingInfo = new BindingInfo()
+                {
+                    BindingSource = BindingSource.Custom,
+
+                    // Simulates [BindProperty] on a parameter
+                    RequestPredicate = ((IRequestPredicateProvider)new BindPropertyAttribute()).RequestPredicate,
+                },
+                ParameterType = typeof(string)
+            });
+
+            actionDescriptor.BoundProperties.Add(new ParameterDescriptor
+            {
+                Name = nameof(TestController.NullableProperty),
+                BindingInfo = new BindingInfo()
+                {
+                    BindingSource = BindingSource.Custom,
+
+                    // Simulates [BindProperty] on a property
+                    RequestPredicate = ((IRequestPredicateProvider)new BindPropertyAttribute()).RequestPredicate,
+                },
+                ParameterType = typeof(string)
+            });
+
+            var controllerContext = GetControllerContext(actionDescriptor);
+            controllerContext.HttpContext.Request.Method = "POST";
+
+            var controller = new TestController();
+            var arguments = new Dictionary<string, object>(StringComparer.Ordinal);
+
+            var binder = new StubModelBinder(ModelBindingResult.Success(model: null));
+            var factory = GetModelBinderFactory(binder);
+            var parameterBinder = GetParameterBinder(factory);
+
+            // Some non default value.
+            controller.NullableProperty = -1;
+
+            // Act
+            var binderDelegate = ControllerBinderDelegateProvider.CreateBinderDelegate(
+                parameterBinder,
+                factory,
+                TestModelMetadataProvider.CreateDefaultProvider(),
+                actionDescriptor);
+
+            await binderDelegate(controllerContext, controller, arguments);
+
+            // Assert
+            Assert.Null(controller.NullableProperty);
+            Assert.Contains("test-parameter", arguments.Keys);
+            Assert.Null(arguments["test-parameter"]);
+        }
+
         // property name, property type, property accessor, input value, expected value
         public static TheoryData<string, Type, Func<object, object>, object, object> SkippedPropertyData
         {

test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs

@@ -728,7 +728,7 @@ public async Task HandlerMethodArgumentsAndPropertiesAreModelBound()
         public async Task PagePropertiesAreNotBoundInGetRequests()
         {
             // Arrange
-            var expected = "Id = 11, Name = , Age =";
+            var expected = "Id = 11, Name = Test-Name, Age =";
             var validationError = "The Name field is required.";
             var request = new HttpRequestMessage(HttpMethod.Get, "Pages/PropertyBinding/PageWithPropertyAndArgumentBinding?id=11")
             {