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.

src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PagesBaseClassAttribute.cs → src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageModelAttribute.cs

@@ -6,11 +6,11 @@
 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 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.
     /// </summary>
-    [AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = false)]
-    public class PagesBaseClassAttribute : Attribute
+    [AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = true)]
+    public class PageModelAttribute : Attribute
     {
     }
 }

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 @@ protected virtual PageApplicationModel CreateModel(
                 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).
-            TypeInfo handlerType;
+            var modelTypeInfo = modelProperty.PropertyType.GetTypeInfo();
 
-            var handlerModels = modelTypeInfo == null ? null : CreateHandlerModels(modelTypeInfo);
-            if (handlerModels?.Count > 0)
+            // Now we want figure out which type is the handler type.
+            TypeInfo handlerType;
+            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 @@ protected virtual PageApplicationModel CreateModel(
                 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 @@ protected virtual PageApplicationModel CreateModel(
         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 @@ internal void PopulateHandlerProperties(PageApplicationModel pageModel)
         }
 
         // Internal for unit testing
-        internal IList<PageHandlerModel> CreateHandlerModels(TypeInfo handlerTypeInfo)
+        internal void PopulateHandlerMethods(PageApplicationModel pageModel)
         {
-            var methods = handlerTypeInfo.GetMethods();
-            var results = new List<PageHandlerModel>();
+            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);
+                }
+            }
         }
 
         /// <summary>
@@ -170,16 +177,6 @@ protected virtual PageHandlerModel CreateHandlerModel(MethodInfo method)
                 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 @@ protected virtual bool IsHandler(MethodInfo methodInfo)
                 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)

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
 {
     /// <summary>
     /// A base class for a Razor page.
     /// </summary>
-    [PagesBaseClass]
     public abstract class Page : PageBase
     {
     }

src/Microsoft.AspNetCore.Mvc.RazorPages/PageBase.cs

@@ -13,7 +13,6 @@
 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
     /// <summary>
     /// A base class for a Razor page.
     /// </summary>
-    [PagesBaseClass]
     public abstract class PageBase : RazorPageBase
     {
         private IObjectModelValidator _objectValidator;

src/Microsoft.AspNetCore.Mvc.RazorPages/PageModel.cs

@@ -21,7 +21,7 @@
 
 namespace Microsoft.AspNetCore.Mvc.RazorPages
 {
-    [PagesBaseClass]
+    [PageModelAttribute]
     public abstract class PageModel
     {
         private IModelMetadataProvider _metadataProvider;

src/Microsoft.AspNetCore.Mvc.RazorPages/Resources.resx

@@ -144,4 +144,10 @@
   <data name="AsyncPageFilter_InvalidShortCircuit" xml:space="preserve">
     <value>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}.</value>
   </data>
+  <data name="InvalidPageType_WrongBase" xml:space="preserve">
+    <value>The type '{0}' is not a valid page. A page must inherit from '{1}'.</value>
+  </data>
+  <data name="InvalidPageType_NoModelProperty" xml:space="preserve">
+    <value>The type '{0}' is not a valid page. A page must define a public, non-static '{1}' property.</value>
+  </data>
 </root>

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

@@ -384,6 +384,19 @@ public async Task HelloWorldWithPageModelHandler_CanGetContent()
             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()
         {

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
@@ -28,12 +30,14 @@ public void OnProvidersExecuting_IgnoresAttributesOnHandlerMethods()
             Assert.Empty(context.PageApplicationModel.Filters);
         }
 
-        private class PageWiithAuthorizeHandlers
+        private class PageWiithAuthorizeHandlers : Page
         {
             public ModelWuthAuthorizeHandlers Model => null;
+
+            public override Task ExecuteAsync() => throw new NotImplementedException();
         }
 
-        public class ModelWuthAuthorizeHandlers
+        public class ModelWuthAuthorizeHandlers : PageModel
         {
             [Authorize]
             public void OnGet()
@@ -58,13 +62,15 @@ public void OnProvidersExecuting_AddsAuthorizeFilter_IfModelHasAuthorizationAttr
                 f => Assert.IsType<AuthorizeFilter>(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 @@ public void OnProvidersExecuting_CollatesAttributesFromInheritedTypes()
             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 @@ public void OnProvidersExecuting_AddsAllowAnonymousFilter()
                 f => Assert.IsType<AllowAnonymousFilter>(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() { }
         }