diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/BindingInfo.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/BindingInfo.cs index 941240cb64..d47cda1bc2 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/BindingInfo.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/BindingInfo.cs @@ -34,6 +34,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding BinderModelName = other.BinderModelName; BinderType = other.BinderType; PropertyFilterProvider = other.PropertyFilterProvider; + RequestPredicate = other.RequestPredicate; } /// @@ -56,6 +57,12 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding /// public IPropertyFilterProvider PropertyFilterProvider { get; set; } + /// + /// Gets or sets a predicate which determines whether or not the model should be bound based on state + /// from the current request. + /// + public Func RequestPredicate { get; set; } + /// /// Constructs a new instance of from the given . /// @@ -113,6 +120,17 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding bindingInfo.PropertyFilterProvider = new CompositePropertyFilterProvider(propertyFilterProviders); } + // RequestPredicate + foreach (var requestPredicateProvider in attributes.OfType()) + { + isBindingInfoPresent = true; + if (requestPredicateProvider.RequestPredicate != null) + { + bindingInfo.RequestPredicate = requestPredicateProvider.RequestPredicate; + break; + } + } + return isBindingInfoPresent ? bindingInfo : null; } diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/IRequestPredicateProvider.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/IRequestPredicateProvider.cs new file mode 100644 index 0000000000..ab72771f22 --- /dev/null +++ b/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 +{ + /// + /// An interface that allows a top-level model to be bound or not bound based on state associated + /// with the current request. + /// + public interface IRequestPredicateProvider + { + /// + /// Gets a function which determines whether or not the model object should be bound based + /// on the current request. + /// + Func RequestPredicate { get; } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/BindPropertyAttribute.cs b/src/Microsoft.AspNetCore.Mvc.Core/BindPropertyAttribute.cs similarity index 59% rename from src/Microsoft.AspNetCore.Mvc.RazorPages/BindPropertyAttribute.cs rename to src/Microsoft.AspNetCore.Mvc.Core/BindPropertyAttribute.cs index 9146b33ec6..5056377712 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/BindPropertyAttribute.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/BindPropertyAttribute.cs @@ -4,11 +4,15 @@ 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 static readonly Func _supportsAllRequests = (c) => true; + + private static readonly Func _supportsNonGetRequests = IsNonGetRequest; + private BindingSource _bindingSource; public bool SupportsGet { get; set; } @@ -35,5 +39,18 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages /// public string Name { get; set; } + + Func IRequestPredicateProvider.RequestPredicate + { + get + { + return SupportsGet ? _supportsAllRequests : _supportsNonGetRequests; + } + } + + private static bool IsNonGetRequest(ActionContext context) + { + return !string.Equals(context.HttpContext.Request.Method, "GET", StringComparison.OrdinalIgnoreCase); + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ParameterBinder.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ParameterBinder.cs index 96b33be59e..cabb45c096 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ParameterBinder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ParameterBinder.cs @@ -152,6 +152,11 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding throw new ArgumentNullException(nameof(metadata)); } + if (parameter.BindingInfo?.RequestPredicate?.Invoke(actionContext) == false) + { + return ModelBindingResult.Failed(); + } + var modelBindingContext = DefaultModelBindingContext.CreateBindingContext( actionContext, valueProvider, diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageBoundPropertyDescriptor.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageBoundPropertyDescriptor.cs index 37f97138f4..56cbdc40f7 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageBoundPropertyDescriptor.cs +++ b/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; } } } diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageLoader.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageLoader.cs index 3c8cfbad72..a606355006 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageLoader.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageLoader.cs @@ -248,26 +248,31 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal 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(); - 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); diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PagePropertyBinderFactory.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PagePropertyBinderFactory.cs index 352fed4a7c..3d6a895e47 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PagePropertyBinderFactory.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PagePropertyBinderFactory.cs @@ -7,7 +7,6 @@ using System.Threading.Tasks; 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 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal IList properties, IList 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) { diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerBinderDelegateProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerBinderDelegateProviderTest.cs index ef3a8a67ea..e28222d178 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerBinderDelegateProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerBinderDelegateProviderTest.cs @@ -13,6 +13,7 @@ using Microsoft.AspNetCore.Mvc.Formatters; 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 @@ namespace Microsoft.AspNetCore.Mvc.Internal 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(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(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, object, object> SkippedPropertyData { diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs index ac243614da..fdbf5893d0 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs @@ -728,7 +728,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests 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") { diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BindPropertyIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BindPropertyIntegrationTest.cs new file mode 100644 index 0000000000..40df2d96b5 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BindPropertyIntegrationTest.cs @@ -0,0 +1,101 @@ +// 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.Collections.Generic; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.Extensions.Primitives; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.IntegrationTests +{ + // Integration tests for binding top level models with [BindProperty] + public class BindPropertyIntegrationTest + { + private class Person + { + public string Name { get; set; } + } + + [Fact] + public async Task BindModelAsync_WithBindProperty_BindsModel_WhenRequestIsNotGet() + { + // Arrange + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var parameter = new ParameterDescriptor() + { + Name = "parameter", + ParameterType = typeof(Person), + BindingInfo = BindingInfo.GetBindingInfo(new[] { new BindPropertyAttribute() }), + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + request.Method = "POST"; + request.QueryString = new QueryString("?parameter.Name=Joey"); + }); + + // Act + var result = await parameterBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(result.IsModelSet); + + Assert.Equal("Joey", Assert.IsType(result.Model).Name); + } + + [Fact] + public async Task BindModelAsync_WithBindProperty_SupportsGet_BindsModel_WhenRequestIsGet() + { + // Arrange + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var parameter = new ParameterDescriptor() + { + Name = "parameter", + ParameterType = typeof(Person), + BindingInfo = BindingInfo.GetBindingInfo(new[] { new BindPropertyAttribute() { SupportsGet = true } }), + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + request.Method = "GET"; + request.QueryString = new QueryString("?parameter.Name=Joey"); + }); + + // Act + var result = await parameterBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(result.IsModelSet); + + Assert.Equal("Joey", Assert.IsType(result.Model).Name); + } + + [Fact] + public async Task BindModelAsync_WithBindProperty_DoesNotBindModel_WhenRequestIsGet() + { + // Arrange + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var parameter = new ParameterDescriptor() + { + Name = "parameter", + ParameterType = typeof(Person), + BindingInfo = BindingInfo.GetBindingInfo(new[] { new BindPropertyAttribute() }), + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + request.Method = "GET"; + request.QueryString = new QueryString("?parameter.Name=Joey"); + }); + + // Act + var result = await parameterBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.False(result.IsModelSet); + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/DefaultPageLoaderTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/DefaultPageLoaderTest.cs index b3a603221c..df27d80246 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/DefaultPageLoaderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/DefaultPageLoaderTest.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Linq; using System.Reflection; using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.ActionConstraints; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure; @@ -576,7 +577,17 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal p => { Assert.Equal("Property", p.Property.Name); - Assert.True(p.SupportsGet); + Assert.NotNull(p.BindingInfo.RequestPredicate); + Assert.True(p.BindingInfo.RequestPredicate(new ActionContext() + { + HttpContext = new DefaultHttpContext() + { + Request = + { + Method ="GET", + } + } + })); }); } @@ -603,7 +614,17 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal p => { Assert.Equal("Property", p.Property.Name); - Assert.True(p.SupportsGet); + Assert.NotNull(p.BindingInfo.RequestPredicate); + Assert.True(p.BindingInfo.RequestPredicate(new ActionContext() + { + HttpContext = new DefaultHttpContext() + { + Request = + { + Method ="GET", + } + } + })); }); } @@ -628,7 +649,17 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal p => { Assert.Equal("Property", p.Property.Name); - Assert.False(p.SupportsGet); + Assert.NotNull(p.BindingInfo.RequestPredicate); + Assert.False(p.BindingInfo.RequestPredicate(new ActionContext() + { + HttpContext = new DefaultHttpContext() + { + Request = + { + Method ="GET", + } + } + })); }); } diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PagePropertyBinderFactoryTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PagePropertyBinderFactoryTest.cs index 771e32de1f..c8a3bdb1f2 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PagePropertyBinderFactoryTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PagePropertyBinderFactoryTest.cs @@ -324,7 +324,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal [InlineData("Get")] [InlineData("GET")] [InlineData("gET")] - public async Task ModelBinderFactory_IgnoresPropertyWithoutSupportsGet_WhenRequestIsGet(string method) + public async Task ModelBinderFactory_BindsPropertyWithoutSupportsGet_WhenRequestIsGet(string method) { // Arrange var type = typeof(PageModelWithSupportsGetProperty).GetTypeInfo(); @@ -338,7 +338,11 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal Name = nameof(PageModelWithSupportsGetProperty.SupportsGet), ParameterType = typeof(string), Property = type.GetProperty(nameof(PageModelWithSupportsGetProperty.SupportsGet)), - SupportsGet = true, + BindingInfo = new BindingInfo() + { + // Simulates placing a [BindProperty] on the property + RequestPredicate = ((IRequestPredicateProvider)new BindPropertyAttribute() { SupportsGet = true }).RequestPredicate, + } }, new PageBoundPropertyDescriptor() { @@ -356,7 +360,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal var binder = new TestParameterBinder(new Dictionary() { { "SupportsGet", "value" }, - { "Default", "ignored" }, + { "Default", "set" }, }); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); @@ -366,12 +370,16 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal { PageContext = new PageContext() { - HttpContext = new DefaultHttpContext(), + HttpContext = new DefaultHttpContext() + { + Request= + { + Method = method, + } + } } }; - page.HttpContext.Request.Method = method; - var model = new PageModelWithSupportsGetProperty(); // Act @@ -379,7 +387,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal // Assert Assert.Equal("value", model.SupportsGet); - Assert.Null(model.Default); + Assert.Equal("set", model.Default); } [Fact] @@ -397,7 +405,10 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal Name = nameof(PageModelWithSupportsGetProperty.SupportsGet), ParameterType = typeof(string), Property = type.GetProperty(nameof(PageModelWithSupportsGetProperty.SupportsGet)), - SupportsGet = true, + BindingInfo = new BindingInfo() + { + RequestPredicate = ((IRequestPredicateProvider)new BindPropertyAttribute() { SupportsGet = true }).RequestPredicate, + } }, new PageBoundPropertyDescriptor() {