From 579aca0121799cca39fcf1a78425cce1f29d0efd Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Wed, 28 Jun 2017 11:17:43 -0700 Subject: [PATCH] Define semantics for pagemodels Fixes #6210 Now a pagemodel requires a [PageModel] somewhere in it's hierarchy. We don't do a guess at whether or not your model class is a PageModel. --- ...lassAttribute.cs => PageModelAttribute.cs} | 8 +- .../DefaultPageApplicationModelProvider.cs | 112 +++--- .../Page.cs | 3 - .../PageBase.cs | 2 - .../PageModel.cs | 2 +- .../Properties/Resources.Designer.cs | 28 ++ .../Resources.resx | 6 + .../RazorPagesTest.cs | 13 + ...izationPageApplicationModelProviderTest.cs | 32 +- ...DefaultPageApplicationModelProviderTest.cs | 334 +++++++++++++----- .../Internal/PocoModel.cs | 17 - .../HelloWorldWithPageModelAttributeModel.cs | 19 + ...lloWorldWithPageModelAttributeModel.cshtml | 3 + .../RazorPagesWebSite/ModelWithPageFilter.cs | 2 +- .../Conventions/AuthFolder/AnonymousModel.cs | 3 +- 15 files changed, 410 insertions(+), 174 deletions(-) rename src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/{PagesBaseClassAttribute.cs => PageModelAttribute.cs} (60%) delete mode 100644 test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PocoModel.cs create mode 100644 test/WebSites/RazorPagesWebSite/HelloWorldWithPageModelAttributeModel.cs create mode 100644 test/WebSites/RazorPagesWebSite/HelloWorldWithPageModelAttributeModel.cshtml diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PagesBaseClassAttribute.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageModelAttribute.cs similarity index 60% rename from src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PagesBaseClassAttribute.cs rename to src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageModelAttribute.cs index bf453ef994..0ae311a8ee 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PagesBaseClassAttribute.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageModelAttribute.cs @@ -6,11 +6,11 @@ using System; namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure { /// - /// An attribute for base classes for Pages and PageModels. Applying this attribute to a type - /// suppresses discovery of handler methods for that type. + /// An attribute for base classes for page models. Applying this attribute to a type + /// marks all subclasses of that type as page model types. /// - [AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = false)] - public class PagesBaseClassAttribute : Attribute + [AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = true)] + public class PageModelAttribute : Attribute { } } diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageApplicationModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageApplicationModelProvider.cs index d14accf218..f253913c93 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageApplicationModelProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageApplicationModelProvider.cs @@ -2,11 +2,11 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Collections.Generic; using System.Reflection; using Microsoft.AspNetCore.Mvc.ApplicationModels; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Mvc.Razor; using Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure; using Microsoft.Extensions.Internal; using Microsoft.Extensions.Options; @@ -66,23 +66,34 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal throw new ArgumentNullException(nameof(pageTypeInfo)); } + if (!typeof(PageBase).GetTypeInfo().IsAssignableFrom(pageTypeInfo)) + { + throw new InvalidOperationException(Resources.FormatInvalidPageType_WrongBase( + pageTypeInfo.FullName, + typeof(PageBase).FullName)); + } + // Pages always have a model type. If it's not set explicitly by the developer using // @model, it will be the same as the page type. - var modelTypeInfo = pageTypeInfo.GetProperty(ModelPropertyName)?.PropertyType?.GetTypeInfo(); + var modelProperty = pageTypeInfo.GetProperty(ModelPropertyName, BindingFlags.Public | BindingFlags.Instance); + if (modelProperty == null) + { + throw new InvalidOperationException(Resources.FormatInvalidPageType_NoModelProperty( + pageTypeInfo.FullName, + ModelPropertyName)); + } - // Now we want to find the handler methods. If the model defines any handlers, then we'll use those, - // otherwise look at the page itself (unless the page IS the model, in which case we already looked). + var modelTypeInfo = modelProperty.PropertyType.GetTypeInfo(); + + // Now we want figure out which type is the handler type. TypeInfo handlerType; - - var handlerModels = modelTypeInfo == null ? null : CreateHandlerModels(modelTypeInfo); - if (handlerModels?.Count > 0) + if (modelProperty.PropertyType.IsDefined(typeof(PageModelAttribute), inherit: true)) { handlerType = modelTypeInfo; } else { - handlerType = pageTypeInfo.GetTypeInfo(); - handlerModels = CreateHandlerModels(pageTypeInfo); + handlerType = pageTypeInfo; } var handlerTypeAttributes = handlerType.GetCustomAttributes(inherit: true); @@ -95,27 +106,9 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal ModelType = modelTypeInfo, }; - for (var i = 0; i < handlerModels.Count; i++) - { - var handlerModel = handlerModels[i]; - handlerModel.Page = pageModel; - pageModel.HandlerMethods.Add(handlerModel); - } - + PopulateHandlerMethods(pageModel); PopulateHandlerProperties(pageModel); - - for (var i = 0; i < _globalFilters.Count; i++) - { - pageModel.Filters.Add(_globalFilters[i]); - } - - for (var i = 0; i < handlerTypeAttributes.Length; i++) - { - if (handlerTypeAttributes[i] is IFilterMetadata filter) - { - pageModel.Filters.Add(filter); - } - } + PopulateFilters(pageModel); return pageModel; } @@ -124,6 +117,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal internal void PopulateHandlerProperties(PageApplicationModel pageModel) { var properties = PropertyHelper.GetVisibleProperties(pageModel.HandlerType.AsType()); + for (var i = 0; i < properties.Length; i++) { var propertyModel = CreatePropertyModel(properties[i].Property); @@ -136,21 +130,34 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal } // Internal for unit testing - internal IList CreateHandlerModels(TypeInfo handlerTypeInfo) + internal void PopulateHandlerMethods(PageApplicationModel pageModel) { - var methods = handlerTypeInfo.GetMethods(); - var results = new List(); + var methods = pageModel.HandlerType.GetMethods(); for (var i = 0; i < methods.Length; i++) { var handler = CreateHandlerModel(methods[i]); if (handler != null) { - results.Add(handler); + pageModel.HandlerMethods.Add(handler); } } + } - return results; + internal void PopulateFilters(PageApplicationModel pageModel) + { + for (var i = 0; i < _globalFilters.Count; i++) + { + pageModel.Filters.Add(_globalFilters[i]); + } + + for (var i = 0; i < pageModel.HandlerTypeAttributes.Count; i++) + { + if (pageModel.HandlerTypeAttributes[i] is IFilterMetadata filter) + { + pageModel.Filters.Add(filter); + } + } } /// @@ -170,16 +177,6 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal return null; } - if (method.IsDefined(typeof(NonHandlerAttribute))) - { - return null; - } - - if (method.DeclaringType.GetTypeInfo().IsDefined(typeof(PagesBaseClassAttribute))) - { - return null; - } - if (!TryParseHandlerMethod(method.Name, out var httpMethod, out var handlerName)) { return null; @@ -294,7 +291,32 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal return false; } - return methodInfo.IsPublic; + if (!methodInfo.IsPublic) + { + return false; + } + + if (methodInfo.IsDefined(typeof(NonHandlerAttribute))) + { + return false; + } + + // Exclude the whole hierarchy of Page. + var declaringType = methodInfo.DeclaringType; + if (declaringType == typeof(Page) || + declaringType == typeof(PageBase) || + declaringType == typeof(RazorPageBase)) + { + return false; + } + + // Exclude methods declared on PageModel + if (declaringType == typeof(PageModel)) + { + return false; + } + + return true; } internal static bool TryParseHandlerMethod(string methodName, out string httpMethod, out string handler) diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Page.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Page.cs index 2faeae6e7a..c6ee14940d 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Page.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Page.cs @@ -1,14 +1,11 @@ // 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 Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure; - namespace Microsoft.AspNetCore.Mvc.RazorPages { /// /// A base class for a Razor page. /// - [PagesBaseClass] public abstract class Page : PageBase { } diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/PageBase.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/PageBase.cs index fee31ea83c..ccb478c495 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/PageBase.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/PageBase.cs @@ -13,7 +13,6 @@ using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.ModelBinding.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Microsoft.AspNetCore.Mvc.Razor; -using Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure; using Microsoft.AspNetCore.Mvc.Rendering; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection; @@ -24,7 +23,6 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages /// /// A base class for a Razor page. /// - [PagesBaseClass] public abstract class PageBase : RazorPageBase { private IObjectModelValidator _objectValidator; diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/PageModel.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/PageModel.cs index a7daf7ba4f..d8083e4393 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/PageModel.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/PageModel.cs @@ -21,7 +21,7 @@ using Microsoft.Net.Http.Headers; namespace Microsoft.AspNetCore.Mvc.RazorPages { - [PagesBaseClass] + [PageModelAttribute] public abstract class PageModel { private IModelMetadataProvider _metadataProvider; diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Properties/Resources.Designer.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Properties/Resources.Designer.cs index f3559d62e7..4e9aa8a0d3 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Properties/Resources.Designer.cs @@ -136,6 +136,34 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages internal static string FormatAsyncPageFilter_InvalidShortCircuit(object p0, object p1, object p2, object p3) => string.Format(CultureInfo.CurrentCulture, GetString("AsyncPageFilter_InvalidShortCircuit"), p0, p1, p2, p3); + /// + /// The type '{0}' is not a valid page. A page must inherit from '{1}'. + /// + internal static string InvalidPageType_WrongBase + { + get => GetString("InvalidPageType_WrongBase"); + } + + /// + /// The type '{0}' is not a valid page. A page must inherit from '{1}'. + /// + internal static string FormatInvalidPageType_WrongBase(object p0, object p1) + => string.Format(CultureInfo.CurrentCulture, GetString("InvalidPageType_WrongBase"), p0, p1); + + /// + /// The type '{0}' is not a valid page. A page must define a public, non-static '{1}' property. + /// + internal static string InvalidPageType_NoModelProperty + { + get => GetString("InvalidPageType_NoModelProperty"); + } + + /// + /// The type '{0}' is not a valid page. A page must define a public, non-static '{1}' property. + /// + internal static string FormatInvalidPageType_NoModelProperty(object p0, object p1) + => string.Format(CultureInfo.CurrentCulture, GetString("InvalidPageType_NoModelProperty"), p0, p1); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Resources.resx b/src/Microsoft.AspNetCore.Mvc.RazorPages/Resources.resx index f85bb00639..6b80508128 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Resources.resx +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Resources.resx @@ -144,4 +144,10 @@ If an {0} provides a result value by setting the {1} property of {2} to a non-null value, then it cannot call the next filter by invoking {3}. + + The type '{0}' is not a valid page. A page must inherit from '{1}'. + + + The type '{0}' is not a valid page. A page must define a public, non-static '{1}' property. + \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs index 6edff8f491..4dc5ed4a40 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs @@ -384,6 +384,19 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests Assert.StartsWith("Hello, pagemodel!", content.Trim()); } + [Fact] + public async Task HelloWorldWithPageModelAttributeHandler() + { + // Arrange + var url = "HelloWorldWithPageModelAttributeModel?message=DecoratedModel"; + + // Act + var content = await Client.GetStringAsync(url); + + // Assert + Assert.Equal("Hello, DecoratedModel!", content.Trim()); + } + [Fact] public async Task PageWithoutContent() { diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/AuthorizationPageApplicationModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/AuthorizationPageApplicationModelProviderTest.cs index ef7cc860c1..7da47e9589 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/AuthorizationPageApplicationModelProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/AuthorizationPageApplicationModelProviderTest.cs @@ -1,11 +1,13 @@ // 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; using System.Reflection; +using System.Threading.Tasks; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc.ApplicationModels; using Microsoft.AspNetCore.Mvc.Authorization; +using Microsoft.AspNetCore.Mvc.Razor; using Xunit; namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal @@ -18,7 +20,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal // Arrange var policyProvider = new DefaultAuthorizationPolicyProvider(new TestOptionsManager()); var autorizationProvider = new AuthorizationPageApplicationModelProvider(policyProvider); - var typeInfo = typeof(PageWiithAuthorizeHandlers).GetTypeInfo(); + var typeInfo = typeof(PageWithAuthorizeHandlers).GetTypeInfo(); var context = GetApplicationProviderContext(typeInfo); // Act @@ -28,12 +30,14 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal Assert.Empty(context.PageApplicationModel.Filters); } - private class PageWiithAuthorizeHandlers + private class PageWithAuthorizeHandlers : Page { - public ModelWuthAuthorizeHandlers Model => null; + public ModelWithAuthorizeHandlers Model => null; + + public override Task ExecuteAsync() => throw new NotImplementedException(); } - public class ModelWuthAuthorizeHandlers + public class ModelWithAuthorizeHandlers : PageModel { [Authorize] public void OnGet() @@ -58,13 +62,15 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal f => Assert.IsType(f)); } - private class TestPage + private class TestPage : Page { public TestModel Model => null; + + public override Task ExecuteAsync() => throw new NotImplementedException(); } [Authorize] - private class TestModel + private class TestModel : PageModel { public virtual void OnGet() { @@ -93,13 +99,15 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal Assert.Equal(3, authorizeFilter.Policy.Requirements.Count); } - private class TestPageWithDerivedModel + private class TestPageWithDerivedModel : Page { public DeriviedModel Model => null; + + public override Task ExecuteAsync() =>throw new NotImplementedException(); } [Authorize(Policy = "Base")] - public class BaseModel + public class BaseModel : PageModel { } @@ -128,13 +136,15 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal f => Assert.IsType(f)); } - private class PageWithAnonymousModel + private class PageWithAnonymousModel : Page { public AnonymousModel Model => null; + + public override Task ExecuteAsync() => throw new NotImplementedException(); } [AllowAnonymous] - public class AnonymousModel + public class AnonymousModel : PageModel { public void OnGet() { } } diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/DefaultPageApplicationModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/DefaultPageApplicationModelProviderTest.cs index d0f3835231..ca4e47583b 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/DefaultPageApplicationModelProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/DefaultPageApplicationModelProviderTest.cs @@ -8,19 +8,135 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.ApplicationModels; using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Mvc.Razor; +using Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure; using Microsoft.Extensions.Options; using Xunit; namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal { - public partial class DefaultPageApplicationModelProviderTest + public class DefaultPageApplicationModelProviderTest { [Fact] - public void OnProvidersExecuting_SetsPageAsHandlerType_IfModelPropertyDoesNotExist() + public void OnProvidersExecuting_ThrowsIfPageDoesNotDeriveFromValidBaseType() { // Arrange var provider = new TestPageApplicationModelProvider(); - var typeInfo = typeof(TestPage).GetTypeInfo(); + var typeInfo = typeof(InvalidPageWithWrongBaseClass).GetTypeInfo(); + var descriptor = new PageActionDescriptor(); + var context = new PageApplicationModelProviderContext(descriptor, typeInfo); + + // Act & Assert + var ex = Assert.Throws(() => provider.OnProvidersExecuting(context)); + + // Assert + Assert.Equal( + $"The type '{typeInfo.FullName}' is not a valid page. A page must inherit from '{typeof(PageBase).FullName}'.", + ex.Message); + } + + private class InvalidPageWithWrongBaseClass : RazorPageBase + { + public override void BeginContext(int position, int length, bool isLiteral) + { + throw new NotImplementedException(); + } + + public override void EndContext() + { + throw new NotImplementedException(); + } + + public override void EnsureRenderedBodyOrSections() + { + throw new NotImplementedException(); + } + + public override Task ExecuteAsync() + { + throw new NotImplementedException(); + } + } + + [Fact] + public void OnProvidersExecuting_ThrowsIfModelPropertyDoesNotExistOnPage() + { + // Arrange + var provider = new TestPageApplicationModelProvider(); + var typeInfo = typeof(PageWithoutModelProperty).GetTypeInfo(); + var descriptor = new PageActionDescriptor(); + var context = new PageApplicationModelProviderContext(descriptor, typeInfo); + + // Act & Assert + var ex = Assert.Throws(() => provider.OnProvidersExecuting(context)); + + // Assert + Assert.Equal( + $"The type '{typeInfo.FullName}' is not a valid page. A page must define a public, non-static 'Model' property.", + ex.Message); + } + + private class PageWithoutModelProperty : PageBase + { + public override Task ExecuteAsync() => throw new NotImplementedException(); + } + + [Fact] + public void OnProvidersExecuting_ThrowsIfModelPropertyIsNotPublic() + { + // Arrange + var provider = new TestPageApplicationModelProvider(); + var typeInfo = typeof(PageWithNonVisibleModel).GetTypeInfo(); + var descriptor = new PageActionDescriptor(); + var context = new PageApplicationModelProviderContext(descriptor, typeInfo); + + // Act & Assert + var ex = Assert.Throws(() => provider.OnProvidersExecuting(context)); + + // Assert + Assert.Equal( + $"The type '{typeInfo.FullName}' is not a valid page. A page must define a public, non-static 'Model' property.", + ex.Message); + } + + private class PageWithNonVisibleModel : PageBase + { + private object Model => null; + + public override Task ExecuteAsync() => throw new NotImplementedException(); + } + + [Fact] + public void OnProvidersExecuting_ThrowsIfModelPropertyIsStatic() + { + // Arrange + var provider = new TestPageApplicationModelProvider(); + var typeInfo = typeof(PageWithStaticModel).GetTypeInfo(); + var descriptor = new PageActionDescriptor(); + var context = new PageApplicationModelProviderContext(descriptor, typeInfo); + + // Act & Assert + var ex = Assert.Throws(() => provider.OnProvidersExecuting(context)); + + // Assert + Assert.Equal( + $"The type '{typeInfo.FullName}' is not a valid page. A page must define a public, non-static 'Model' property.", + ex.Message); + } + + private class PageWithStaticModel : PageBase + { + public static object Model => null; + + public override Task ExecuteAsync() => throw new NotImplementedException(); + } + + [Fact] + public void OnProvidersExecuting_DiscoversPropertiesFromPage_IfModelTypeDoesNotHaveAttribute() + { + // Arrange + var provider = new TestPageApplicationModelProvider(); + var typeInfo = typeof(PageWithModelWithoutPageModelAttribute).GetTypeInfo(); var descriptor = new PageActionDescriptor(); var context = new PageApplicationModelProviderContext(descriptor, typeInfo); @@ -29,49 +145,53 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal // Assert Assert.NotNull(context.PageApplicationModel); - Assert.Same(context.PageApplicationModel.PageType, context.PageApplicationModel.HandlerType); + var propertiesOnPage = context.PageApplicationModel.HandlerProperties + .Where(p => p.PropertyInfo.DeclaringType.GetTypeInfo() == typeInfo); + Assert.Collection( + propertiesOnPage.OrderBy(p => p.PropertyName), + property => + { + Assert.Equal(typeInfo.GetProperty(nameof(PageWithModelWithoutPageModelAttribute.Model)), property.PropertyInfo); + Assert.Equal(nameof(PageWithModelWithoutPageModelAttribute.Model), property.PropertyName); + }, + property => + { + Assert.Equal(typeInfo.GetProperty(nameof(PageWithModelWithoutPageModelAttribute.Property1)), property.PropertyInfo); + Assert.Null(property.BindingInfo); + Assert.Equal(nameof(PageWithModelWithoutPageModelAttribute.Property1), property.PropertyName); + }, + property => + { + Assert.Equal(typeInfo.GetProperty(nameof(PageWithModelWithoutPageModelAttribute.Property2)), property.PropertyInfo); + Assert.Equal(nameof(PageWithModelWithoutPageModelAttribute.Property2), property.PropertyName); + Assert.NotNull(property.BindingInfo); + Assert.Equal(BindingSource.Path, property.BindingInfo.BindingSource); + }); + } + + private class PageWithModelWithoutPageModelAttribute : Page + { + public string Property1 { get; set; } + + [FromRoute] + public object Property2 { get; set; } + + public ModelWithoutPageModelAttribute Model => null; + + public override Task ExecuteAsync() => throw new NotImplementedException(); + } + + private class ModelWithoutPageModelAttribute + { } [Fact] - public void OnProvidersExecuting_SetsPageAsHandlerType_IfModelTypeDoesNotHaveAnyHandlers() + public void OnProvidersExecuting_DiscoversPropertiesFromPageModel_IfModelHasAttribute() { // Arrange var provider = new TestPageApplicationModelProvider(); - var typeInfo = typeof(PageWithModelWithoutHandlers).GetTypeInfo(); - var descriptor = new PageActionDescriptor(); - var context = new PageApplicationModelProviderContext(descriptor, typeInfo); - - // Act - provider.OnProvidersExecuting(context); - - // Assert - Assert.NotNull(context.PageApplicationModel); - Assert.Same(context.PageApplicationModel.PageType, context.PageApplicationModel.HandlerType); - } - - [Fact] - public void OnProvidersExecuting_SetsModelAsHandlerType() - { - // Arrange - var provider = new TestPageApplicationModelProvider(); - var typeInfo = typeof(PageWithModel).GetTypeInfo(); - var descriptor = new PageActionDescriptor(); - var context = new PageApplicationModelProviderContext(descriptor, typeInfo); - - // Act - provider.OnProvidersExecuting(context); - - // Assert - Assert.NotNull(context.PageApplicationModel); - Assert.Same(typeof(TestPageModel).GetTypeInfo(), context.PageApplicationModel.HandlerType); - } - - [Fact] - public void OnProvidersExecuting_DiscoversPropertiesFromPage() - { - // Arrange - var provider = new TestPageApplicationModelProvider(); - var typeInfo = typeof(TestPage).GetTypeInfo(); + var typeInfo = typeof(PageWithModelWithPageModelAttribute).GetTypeInfo(); + var modelType = typeof(ModelWithPageModelAttribute); var descriptor = new PageActionDescriptor(); var context = new PageApplicationModelProviderContext(descriptor, typeInfo); @@ -84,19 +204,31 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal context.PageApplicationModel.HandlerProperties.OrderBy(p => p.PropertyName), property => { - Assert.Equal(typeInfo.GetProperty(nameof(TestPage.Property1)), property.PropertyInfo); - Assert.Null(property.BindingInfo); - Assert.Equal(nameof(TestPage.Property1), property.PropertyName); - }, - property => - { - Assert.Equal(typeInfo.GetProperty(nameof(TestPage.Property2)), property.PropertyInfo); - Assert.Equal(nameof(TestPage.Property2), property.PropertyName); + Assert.Equal(modelType.GetProperty(nameof(ModelWithPageModelAttribute.Property)), property.PropertyInfo); + Assert.Equal(nameof(ModelWithPageModelAttribute.Property), property.PropertyName); Assert.NotNull(property.BindingInfo); Assert.Equal(BindingSource.Path, property.BindingInfo.BindingSource); }); } + private class PageWithModelWithPageModelAttribute : Page + { + public string Property1 { get; set; } + + [FromRoute] + public object Property2 { get; set; } + + public ModelWithPageModelAttribute Model => null; + public override Task ExecuteAsync() => throw new NotImplementedException(); + } + + [PageModel] + private class ModelWithPageModelAttribute + { + [FromRoute] + public string Property { get; set; } + } + [Fact] public void OnProvidersExecuting_DiscoversHandlersFromPage() { @@ -237,7 +369,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal var pageModel = context.PageApplicationModel; Assert.Empty(pageModel.HandlerProperties.Where(p => p.BindingInfo != null)); Assert.Empty(pageModel.HandlerMethods); - Assert.Same(typeof(EmptyPageWithPageModel).GetTypeInfo(), pageModel.HandlerType); + Assert.Same(typeof(EmptyPageModel).GetTypeInfo(), pageModel.HandlerType); Assert.Same(typeof(EmptyPageModel).GetTypeInfo(), pageModel.ModelType); Assert.Same(typeof(EmptyPageWithPageModel).GetTypeInfo(), pageModel.PageType); } @@ -304,9 +436,9 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal // Assert var pageModel = context.PageApplicationModel; - Assert.Collection( + Assert.Contains( pageModel.HandlerProperties, - p => Assert.Equal(modelType.GetProperty(nameof(ModelWithHandler.BindMe)), p.PropertyInfo)); + p => p.PropertyInfo == modelType.GetProperty(nameof(ModelWithHandler.BindMe))); Assert.Collection( pageModel.HandlerMethods, @@ -317,7 +449,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal Assert.Same(typeof(PageWithHandlerThatGetsIgnored).GetTypeInfo(), pageModel.PageType); } - private class ModelWithHandler + private class ModelWithHandler : PageModel { [ModelBinder] public int BindMe { get; set; } @@ -325,7 +457,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal public void OnGet() { } } - private class PageWithHandlerThatGetsIgnored + private class PageWithHandlerThatGetsIgnored : Page { public ModelWithHandler Model => null; @@ -333,11 +465,12 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal public int IgnoreMe { get; set; } public void OnPost() { } + + public override Task ExecuteAsync() => throw new NotImplementedException(); } - - [Fact] // If the model has no handler methods, we look at the page instead. - public void OnProvidersExecuting_FindsHandlerMethodOnPage_WhenModelHasNoHandlers() + [Fact] // If the model does not have the PageModelAttribute, we look at the page instead. + public void OnProvidersExecuting_FindsHandlerMethodOnPage_WhenModelIsNotAnnotatedWithPageModelAttribute() { // Arrange var provider = new TestPageApplicationModelProvider(); @@ -349,8 +482,10 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal // Assert var pageModel = context.PageApplicationModel; + var propertiesOnPage = pageModel.HandlerProperties + .Where(p => p.PropertyInfo.DeclaringType.GetTypeInfo() == typeInfo); Assert.Collection( - pageModel.HandlerProperties.OrderBy(p => p.PropertyName), + propertiesOnPage.OrderBy(p => p.PropertyName), p => Assert.Equal(typeInfo.GetProperty(nameof(PageWithHandler.BindMe)), p.PropertyInfo), p => Assert.Equal(typeInfo.GetProperty(nameof(PageWithHandler.Model)), p.PropertyInfo)); @@ -363,7 +498,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal Assert.Same(typeof(PageWithHandler).GetTypeInfo(), pageModel.PageType); } - private class PageWithHandler + private class PageWithHandler : Page { public PocoModel Model => null; @@ -371,22 +506,36 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal public int BindMe { get; set; } public void OnGet() { } + + public override Task ExecuteAsync() => throw new NotImplementedException(); + } + + private class PocoModel + { + // Just a plain ol' model, nothing to see here. + + [ModelBinder] + public int IgnoreMe { get; set; } + + public void OnGet() { } } [Fact] - public void CreateHandlerModels_DiscoversHandlersFromBaseType() + public void PopulateHandlerMethods_DiscoversHandlersFromBaseType() { // Arrange var provider = new TestPageApplicationModelProvider(); var typeInfo = typeof(InheritsMethods).GetTypeInfo(); var baseType = typeof(TestSetPageModel); + var pageModel = new PageApplicationModel(new PageActionDescriptor(), typeInfo, new object[0]); // Act - var handlerModels = provider.CreateHandlerModels(typeInfo); + provider.PopulateHandlerMethods(pageModel); // Assert + var handlerMethods = pageModel.HandlerMethods; Assert.Collection( - handlerModels.OrderBy(h => h.MethodInfo.DeclaringType.Name).ThenBy(h => h.MethodInfo.Name), + handlerMethods.OrderBy(h => h.MethodInfo.DeclaringType.Name).ThenBy(h => h.MethodInfo.Name), handler => { Assert.Equal(nameof(InheritsMethods.OnGet), handler.MethodInfo.Name); @@ -423,18 +572,20 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal } [Fact] - public void CreateHandlerModels_IgnoresNonPublicMethods() + public void PopulateHandlerMethods_IgnoresNonPublicMethods() { // Arrange var provider = new TestPageApplicationModelProvider(); var typeInfo = typeof(ProtectedModel).GetTypeInfo(); var baseType = typeof(TestSetPageModel); + var pageModel = new PageApplicationModel(new PageActionDescriptor(), typeInfo, new object[0]); // Act - var handlerModels = provider.CreateHandlerModels(typeInfo); + provider.PopulateHandlerMethods(pageModel); // Assert - Assert.Empty(handlerModels); + var handlerMethods = pageModel.HandlerMethods; + Assert.Empty(handlerMethods); } private class ProtectedModel @@ -449,17 +600,19 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal } [Fact] - public void CreateHandlerModels_IgnoreGenericTypeParameters() + public void PopulateHandlerMethods_IgnoreGenericTypeParameters() { // Arrange var provider = new TestPageApplicationModelProvider(); var typeInfo = typeof(GenericClassModel).GetTypeInfo(); + var pageModel = new PageApplicationModel(new PageActionDescriptor(), typeInfo, new object[0]); // Act - var handlerModels = provider.CreateHandlerModels(typeInfo); + provider.PopulateHandlerMethods(pageModel); // Assert - Assert.Empty(handlerModels); + var handlerMethods = pageModel.HandlerMethods; + Assert.Empty(handlerMethods); } private class GenericClassModel @@ -470,19 +623,21 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal } [Fact] - public void CreateHandlerModels_IgnoresStaticMethods() + public void PopulateHandlerMethods_IgnoresStaticMethods() { // Arrange var provider = new TestPageApplicationModelProvider(); var typeInfo = typeof(PageModelWithStaticHandler).GetTypeInfo(); var expected = typeInfo.GetMethod(nameof(PageModelWithStaticHandler.OnGet), BindingFlags.Public | BindingFlags.Instance); + var pageModel = new PageApplicationModel(new PageActionDescriptor(), typeInfo, new object[0]); // Act - var handlerModels = provider.CreateHandlerModels(typeInfo); + provider.PopulateHandlerMethods(pageModel); // Assert + var handlerMethods = pageModel.HandlerMethods; Assert.Collection( - handlerModels, + handlerMethods, handler => Assert.Same(expected, handler.MethodInfo)); } @@ -498,19 +653,21 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal } [Fact] - public void CreateHandlerModels_IgnoresAbstractMethods() + public void PopulateHandlerMethods_IgnoresAbstractMethods() { // Arrange var provider = new TestPageApplicationModelProvider(); var typeInfo = typeof(PageModelWithAbstractMethod).GetTypeInfo(); var expected = typeInfo.GetMethod(nameof(PageModelWithAbstractMethod.OnGet), BindingFlags.Public | BindingFlags.Instance); + var pageModel = new PageApplicationModel(new PageActionDescriptor(), typeInfo, new object[0]); // Act - var handlerModels = provider.CreateHandlerModels(typeInfo); - + provider.PopulateHandlerMethods(pageModel); + // Assert + var handlerMethods = pageModel.HandlerMethods; Assert.Collection( - handlerModels, + handlerMethods, handler => Assert.Same(expected, handler.MethodInfo)); } @@ -524,19 +681,21 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal } [Fact] - public void CreateHandlerModels_IgnoresMethodWithNonHandlerAttribute() + public void PopulateHandlerMethods_IgnoresMethodWithNonHandlerAttribute() { // Arrange var provider = new TestPageApplicationModelProvider(); var typeInfo = typeof(PageWithNonHandlerMethod).GetTypeInfo(); var expected = typeInfo.GetMethod(nameof(PageWithNonHandlerMethod.OnGet), BindingFlags.Public | BindingFlags.Instance); + var pageModel = new PageApplicationModel(new PageActionDescriptor(), typeInfo, new object[0]); // Act - var handlerModels = provider.CreateHandlerModels(typeInfo); + provider.PopulateHandlerMethods(pageModel); // Assert + var handlerMethods = pageModel.HandlerMethods; Assert.Collection( - handlerModels, + handlerMethods, handler => Assert.Same(expected, handler.MethodInfo)); } @@ -558,13 +717,15 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal // Arrange var provider = new TestPageApplicationModelProvider(); var typeInfo = typeof(PageModelWithHandlerNames).GetTypeInfo(); + var pageModel = new PageApplicationModel(new PageActionDescriptor(), typeInfo, new object[0]); // Act - var handlerModels = provider.CreateHandlerModels(typeInfo); + provider.PopulateHandlerMethods(pageModel); // Assert + var handlerMethods = pageModel.HandlerMethods; Assert.Collection( - handlerModels.OrderBy(h => h.MethodInfo.Name), + handlerMethods.OrderBy(h => h.MethodInfo.Name), handler => { Assert.Same(typeInfo.GetMethod(nameof(PageModelWithHandlerNames.OnPutDeleteAsync)), handler.MethodInfo); @@ -591,12 +752,14 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal var provider = new TestPageApplicationModelProvider(); var typeInfo = typeof(PageWithHandlerParameters).GetTypeInfo(); var expected = typeInfo.GetMethod(nameof(PageWithHandlerParameters.OnPost)); + var pageModel = new PageApplicationModel(new PageActionDescriptor(), typeInfo, new object[0]); // Act - var handlerModels = provider.CreateHandlerModels(typeInfo); + provider.PopulateHandlerMethods(pageModel); // Assert - var handler = Assert.Single(handlerModels); + var handlerMethods = pageModel.HandlerMethods; + var handler = Assert.Single(handlerMethods); Assert.Collection( handler.Parameters, @@ -743,14 +906,6 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal } } - private class TestPage - { - public string Property1 { get; set; } - - [FromRoute] - public object Property2 { get; set; } - } - private class PageWithModelWithoutHandlers : Page { public ModelWithoutHandler Model { get; } @@ -775,6 +930,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal public override Task ExecuteAsync() => throw new NotImplementedException(); } + [PageModel] private class TestPageModel { public string Property1 { get; set; } diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PocoModel.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PocoModel.cs deleted file mode 100644 index 1c33b0a5bc..0000000000 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PocoModel.cs +++ /dev/null @@ -1,17 +0,0 @@ -// 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. - - -namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal -{ - public partial class DefaultPageApplicationModelProviderTest - { - private class PocoModel - { - // Just a plain ol' model, nothing to see here. - - [ModelBinder] - public int IgnoreMe { get; set; } - } - } -} diff --git a/test/WebSites/RazorPagesWebSite/HelloWorldWithPageModelAttributeModel.cs b/test/WebSites/RazorPagesWebSite/HelloWorldWithPageModelAttributeModel.cs new file mode 100644 index 0000000000..4a81b6cac0 --- /dev/null +++ b/test/WebSites/RazorPagesWebSite/HelloWorldWithPageModelAttributeModel.cs @@ -0,0 +1,19 @@ +// 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 Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure; + +namespace RazorPagesWebSite +{ + [PageModel] + public class HelloWorldWithPageModelAttributeModel + { + public string Message { get; set; } + + public void OnGet(string message) + { + Message = message; + } + } +} diff --git a/test/WebSites/RazorPagesWebSite/HelloWorldWithPageModelAttributeModel.cshtml b/test/WebSites/RazorPagesWebSite/HelloWorldWithPageModelAttributeModel.cshtml new file mode 100644 index 0000000000..23197b2dbf --- /dev/null +++ b/test/WebSites/RazorPagesWebSite/HelloWorldWithPageModelAttributeModel.cshtml @@ -0,0 +1,3 @@ +@page +@model RazorPagesWebSite.HelloWorldWithPageModelAttributeModel +Hello, @Model.Message! diff --git a/test/WebSites/RazorPagesWebSite/ModelWithPageFilter.cs b/test/WebSites/RazorPagesWebSite/ModelWithPageFilter.cs index f56b01d629..1d2d85c3eb 100644 --- a/test/WebSites/RazorPagesWebSite/ModelWithPageFilter.cs +++ b/test/WebSites/RazorPagesWebSite/ModelWithPageFilter.cs @@ -10,7 +10,7 @@ using Microsoft.AspNetCore.Mvc.RazorPages; namespace RazorPagesWebSite { [HandlerChangingPageFilter] - public class ModelWithPageFilter + public class ModelWithPageFilter : PageModel { public string Message { get; private set; } diff --git a/test/WebSites/RazorPagesWebSite/Pages/Conventions/AuthFolder/AnonymousModel.cs b/test/WebSites/RazorPagesWebSite/Pages/Conventions/AuthFolder/AnonymousModel.cs index fa96abbdbf..3ba2e4605b 100644 --- a/test/WebSites/RazorPagesWebSite/Pages/Conventions/AuthFolder/AnonymousModel.cs +++ b/test/WebSites/RazorPagesWebSite/Pages/Conventions/AuthFolder/AnonymousModel.cs @@ -2,11 +2,12 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Mvc.RazorPages; namespace RazorPagesWebSite { [AllowAnonymous] - public class AnonymousModel + public class AnonymousModel : PageModel { public void OnGet() {