-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Introducing IPageModelActivatorProvider #5712
Conversation
namespace Microsoft.AspNetCore.Mvc.RazorPages | ||
{ | ||
/// <summary> | ||
/// Provides methods for creation and disposal of Razor page models. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The P
should be capitalized because it's a proper name #JustEilonThings
using Microsoft.AspNetCore.Mvc.Internal; | ||
using Microsoft.Extensions.DependencyInjection; | ||
|
||
namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we have a .Infrastructure
namespace already parked it would be good to put this there and document it for extensibility. It was common in the past to subclass our factories.
} | ||
|
||
var factory = ActivatorUtilities.CreateFactory(modelTypeInfo, Type.EmptyTypes); | ||
return (context) => factory(context.HttpContext.RequestServices, EmptyArray<object>.Instance); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be dope to put this part into a virtual method, then there's less boilerplate if you want to use something other than ActivatorUtilities
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, my brain is crazy. Ignore this comment as soon as possible
/// the model instance. The property must have a public set method. | ||
/// </summary> | ||
[AttributeUsage(AttributeTargets.Property, AllowMultiple = false, Inherited = true)] | ||
public class PageContextAttribute : Attribute |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this go in .Infrastructure
? Do whatever we did with ActionContextAttribute
.
/// <summary> | ||
/// <see cref="IPageActivatorProvider"/> that uses type activation to create Pages. | ||
/// </summary> | ||
public class DefaultPageModelActivatorProvider : IPageModelActivatorProvider |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll also need the equivalent plumbing for the DI versions of these. Please open an issue for that or do it right away 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good
Fixes #5480