MVC: Changes to [Activate] in beta-5

[rynowak]

Hi all, I wanted to share with you some of the changes that are coming for beta-5 of MVC and some of the rationale about why we're changing [Activate]

What's changing

We're removing [Activate] totally and simplifying some of the things it was possible to accomplish using it. Where [Activate] was used to access context-objects from the current request, it will be replaced by bespoke attributes to handle the supported cases. We've also trimming down the number of context-objects that are available so that it's clear what is supported. Where [Activate] was used to access items from the HttpContext.RequestServices (DI), use constructor injection in its place.

[FromServices] is still supported in controllers on action parameters, public controller properties and model properties. There are no changes to [FromServices] in beta-5.

There are no changes to @inject for beta-5.

Overall there should be very little impact on MVC code unless you're making very heavy use of POCO-style controllers and view components.

POCO Controller Sample

Before:

public class MyController
{
    [Activate]
    public ActionContext ActionContext { get; set; }

    [Activate]
    public HttpContext HttpContext { get; set; }
}

After:

public class MyController
{
    [ActionContext]
    public ActionContext ActionContext { get; set; }

    public HttpContext HttpContext => ActionContext.HttpContext
}

POCO ViewComponent Sample

Before:

public class MyViewComponent
{
    [Activate]
    public IRespository Repository { get; set; }

    [Activate]
    public ViewContext ViewContext { get; set; }
}

After:

public class MyViewComponent
{
    public MyViewComponent(IRepository repository)
    {
        Repository = repository;
    }

    public IRespository Repository { get; }

    [ViewComponentContext]
    public ViewComponentContext ViewComponentContext { get; set; }
}

TagHelper Sample

Before:

public class MyTagHelper : TagHelper
{
    [HtmlAttributeNotBound]
    [Activate]
    public IHtmlEncoder Encoder { get; set; }

    [HtmlAttributeNotBound]
    [Activate]
    public ViewContext ViewContext { get; set; }
}

After:

public class MyTagHelper : TagHelper
{
    public MyTagHelper(IHtmlEncoder encoder)
    {
        Encoder = encoder;
    }

    public IHtmlEncoder Encoder { get; }

    [HtmlAttributeNotBound]
    [ViewContext]
    public ViewContext ViewContext { get; set; }
}

About [Activate]

We originally created [Activate] to solve an implementation problem - how do you get ActionContext and other things into a controller without requiring the user to do this?

public class MyController : Controller
{
    public MyController(ActionContext context)
        : base(context)
    { }
}

We want writing controllers to have the minimum of boilerplate possible, so we created an attribute ([Activate]) that would tell the controller factory to inject certain known types. When we introduced new concepts (TagHelpers, ViewComponents), [Activate] came along for the ride to get the required context into those types.

Once we added the ability to inject arbitrary types from DI with [Activate], we realized that we'd built a secondary DI system that was specific to MVC and that only applied to a few concepts in the framework.

Our goal had always been to play nicely with 3rd party DI so that we could enable the use of the advanced concepts that dedicated DI systems can provide. Each piece of functionality we layer on top is another puzzle to solve for integration with a system like Ninject or Autofac (or your container of choice). We'd rather reset this and build towards a good integrated experience.

We've heard plenty of feedback on aspnet/Mvc#2151 asking us to embrace a merger between DI concepts and [Activate]. Our decision for now is that if there's a significant demand for more powerful DI features, then we should prioritize hooking up existing 3rd party DI solutions to fill the gap. We've done some enabling work for using DI to construct controllers, we might need to go further.

About [FromServices]

This is something we invented when revisiting model binding, and really grew independently of [Activate]. We've heard feedback in the context of the [Activate]/@inject discussion that [FromServices] is a bit awkward and oddball. We're still listening, but have chosen to do nothing for this release.

About @inject

We really feel like @inject is doing what we want it to from a usability standpoint. We heard the feedback loud and clear that everyone is happy with it, so it's staying put.

We don't feel like the many of the concerns we have about [Activate] can be applied to Razor code. Razor has a unique programming style and wouldn't support configuring many of the advanced DI features anyway (since you're not declaring constructors and properties).

The Future

We're still listening to feedback about this even though we're moving in a different direction. We want DI to be part of the Asp.Net platform as a whole, and don't want MVC to have a 'quirks mode' for certain framework pieces. Nothing here is set in stone, this is an iteration and it's still a beta release.

We'd love to see code samples of things you think are too hard or too verbose to do with these changes.

[justinvp]

Minor typo above:

    public MyTagHelper(IHtmlEncoder encoder)
    {
        Repository = repository;
    }

Should be:

    public MyTagHelper(IHtmlEncoder encoder)
    {
        Encoder = encoder;
    }

[rynowak]

Fixed, thanks 😀

[RickyLin]

"The are no changes to @Inject for beta-5"
or
"There are no changes to @Inject for beta-5"?

[rynowak]

I clearly need a better proofreader.

There are no changes to @inject for beta-5

Correct

[glen-84]

Like I said here, it reads as very repetitive – ViewContext written three times.

If the contexts are supplied from outside of the DI system, why not continue to use the single attribute, but limit its use to supplying framework contexts only? I don't think that the term Activate has any ties to DI, but if it does, you could use something else, like [Provision].

If it is technically possible to insert and retrieve these objects from the DI container, then this attribute is not required at all, and a single [Inject] attribute could be used everywhere (I assume it would be possible to replace this attribute with the attribute supplied with one's DI framework of choice?).

[michaldudak]

Is there any reason why every dependency couldn't just be injected into a constructor?

[laurentkempe]

@michaldudak I was about to ask exactly the same question! To me it shows clearly the dependencies.

[kevinkuszyk]

+1 for using just using constructor injection and dropping the attributes.

[m0sa]

+1 @kevinkuszyk

[ghost]

+1 @kevinkuszyk

[ghost]

+1 @kevinkuszyk

[blobor]

+1 @kevinkuszyk

[marcodiniz]

+1 for using just using constructor injection and dropping the
attributes.[2]

or i just didin´t what is the advantage

2015-05-22 8:56 GMT-03:00 Kevin Kuszyk [email protected]:

+1 for using just using constructor injection and dropping the attributes.

—
Reply to this email directly or view it on GitHub
#28 (comment)
.

[henkmollema]

Was wondering the same thing as @michaldudak. Especially this 'hybrid-mode' might look odd:

public class MyTagHelper : TagHelper
{
    public MyTagHelper(IHtmlEncoder encoder)
    {
        Encoder = encoder;
    }

    public IHtmlEncoder Encoder { get; }

    [HtmlAttributeNotBound]
    [ViewContext]
    public ViewContext ViewContext { get; set; }
}

[glen-84]

Isn't it because the context(s) are not "primary" dependencies? In the controller example, you'd rather use constructor injection for your services and/or DbContexts, and get the framework stuff out of the way.

It's this:

public class MyController
{
    public ActionContext ActionContext { get; set; }

    public MyController(MyService service, ActionContext context) { /* ... */ }
}

Versus this:

public class MyController
{
    [ActionContext] or [Inject]
    public ActionContext ActionContext { get; set; }

    public MyController(MyService service) { /* ... */ }
}

Also, if you use the former, you'd expect both arguments to come from the DI container, but only one of them actually is (as far as I understand it).

This would be a lot simpler if the contexts could come from the DI container. That way, the developer can choose how he/she wants them injected.

[CoreyKaylor]

If it's a dependency necessary for the component to work it should come through the constructor in my opinion. This categorization of dependencies of ctor vs. property injection just adds to confusion down the road.

This is somewhat related to this issue regarding per request services.
aspnet/DependencyInjection#237

[Eilon]

The items that are provider by [ViewContext] are not services, and don't come from the DI container, so that's why they are a separate attribute. Your Ninject, Autofact, etc. has no idea what ViewData is. Part of the issue before was conflating "all injected things" as one big bucket, where in reality just because something "comes from somewhere else" doesn't mean it's DI.

[terryt2000]

@Eilon, that's what I was thinking, just not able to articulate it...they are different concerns managed by different "containers"

[michaldudak]

IMO it would be great if the programmer won't have to think if a particular thing should be injected into a constructor or activated some other way. But I do understand this may be impossible/too hard to implement. If, however, we have to use attributes, I'll vote against specific ones like ActionContextAttribute as having the same identifier repeated three times (with three different meanings) just doesn't look right to me.

[mdekrey]

I think this is why many of the existing DI frameworks (Ninject, Unity, Autofac) support adding additional bindings to child scopes, rather than just registering them all up front. A developer shouldn't be concerned with where a value is coming from when setting up an object; isn't that one of the major pillars of DI?

Also, +1 for using constructor injection for everything, or adding attribute injection for if you really don't want child objects to have to worry about it.

[Eilon]

@michaldudak it's not quite the same identifier three times, though it certainly appears that way. (BTW I'm not saying it isn't confusing!) The attribute is really ActionContextAttribute. The type of the property is indeed ActionContext. And the property name is whatever you want, but logically it's normally called ActionContext as well. So while it does appear verbose, there's a fairly good reason for each of those.

@mdekrey the key difference here is that we really, really, really don't want to conflate DI with "other stuff." It's certainly possible for us to do that with some trickery, but that would make things so much harder to debug, so much harder to figure out what's going on, etc.

Please remember that most developers won't see this, and those that do, won't see it very often. These attributes are mostly needed when extending MVC 6, or when using POCO objects (e.g. a POCO Controller, POCO ViewComponent). I believe that relatively few developers will use these scenarios.

[johncrn]

+1 for DI all the things.

Things will get out of hand down the track if you stick with multiple dependency injection strategies. The lines will be blurred between self implemented and framework dependencies and the poor sod trying to put together their solution will be most confused.

@Eilon Could you please explain a little more the difference as you see it between DI and "other stuff"? From the examples shown above the only distinction I can see is that the DI'd services may pertain to a local solution and the attribute injected services are part of the framework implementation. On the face of it I can't see why you'd want a distinction between the two.

[rajajhansi]

If I only care about using Controllers to build Web APIs, not use Razor, ViewComponents and TagHelpers, do I need to worry about any of these attributes? If I only need access to HttpContext, can I not inject IHttpContextAccessor from Hosting? If someone uses POCO for a controller or ViewComponent, the idea is to keep them in a separate class library without any dependencies to frameworks like MVC so they can reuse them in other frameworks like Nancy etc. Adding an attribute pollutes that POCO anyways by adding dependency to a particular framework.
+1 for just sticking to one attribute which should inject one context that provides access to other contexts as properties for those who want to write POCOs (for Controllers, ViewComponents, TagHelpers) yet need access to MVC framework related contexts. Adding one attribute for each context is not good.
+1 for constructor injection only if these contexts can be mocked to run unit tests on controllers. Otherwise, they will grow up like the old ASP.NET HttpContext.

[atrauzzi]

tldr: 👍 to DI all the things. And more.

Few things I'm noticing that would be concerning for anyone who has to both grock and author ASP.NET code (perhaps not always in that order). Most importantly: Having any more than one annotation for any type of injection just seems plain confusing.

I'm going to pick up on @rynowak's handy examples though and refactor them to what I feel would make the most sense. I'll leave the original "befores", and substitute in my own "afters".

POCO Controller Sample

Before:

public class MyController
{
    [Activate]
    public ActionContext ActionContext { get; set; }

    [Activate]
    public HttpContext HttpContext { get; set; }
}

After:

public class MyController
{
    [Inject]
    public ActionContext ActionContext { get; set; }

    // Doing this would be down to taste, good either way.
    public HttpContext HttpContext => ActionContext.HttpContext
}

POCO ViewComponent Sample

Before:

public class MyViewComponent
{
    [Activate]
    public IRepository Repository { get; set; }

    [Activate]
    public ViewContext ViewContext { get; set; }
}

After:

public class MyViewComponent
{
    // Constructor injection is fine.
    public MyViewComponent(IRepository repository)
    {
        Repository = repository;
    }

    // Property injection is fine too, mix it up! 
    [Inject]
    public IRepository Repository { get; }

    [Inject]
    public ViewComponentContext ViewComponentContext { get; set; }
}

TagHelper Sample

Before:

public class MyTagHelper : TagHelper
{
    [HtmlAttributeNotBound]
    [Activate]
    public IHtmlEncoder Encoder { get; set; }

    [HtmlAttributeNotBound]
    [Activate]
    public ViewContext ViewContext { get; set; }
}

After:

public class MyTagHelper : TagHelper
{
    public MyTagHelper(IHtmlEncoder encoder)
    {
        Encoder = encoder;
    }

    // Again, like before, constructor or attribute, developer's choice!
    [Inject]
    public IHtmlEncoder Encoder { get; }

    [HtmlAttributeNotBound]
    [Inject]
    public ViewContext ViewContext { get; set; }
}

Lots of this obviously comes down to removing the pseudo-DI system, DRY, encapsulation and avoiding boilerplate. Based on my examples - especially when you only use constructor or property injection - you will see that they represent a minimum of code.

This is more than just code-golfing.

Improving the naming and forcing first-class mechanisms to apply throughout the framework are a matter of developer-joy. When 99% of questions can be answered with "just inject it", you create a more scrutable system and reduce the amount of magic people have to memorize. This is in contrast to "Did you annotate your ActionContext with [ActionContext]?" Perhaps obvious to the initiated, but the developer was set up to fail in that situation. The framework is forcing them to worry about internals instead of their own code.

For property injection, I think it's important this be available in vanilla ASP.NET and not left up to 3rd party containers. This actually has a much broader impact at a community level than might seem apparent at first. Also, Because ASP.NET is itself just a bunch of packages, the core classes become easier to use with a minimum of project-level code.
Again, not code-golfing so much as expecting developers to be fluent in your internal lifecycle: "Did you call the parent constructor?!" - things like this just turn people off and show poorly in books and demos.

---

The last thing that I would hate to see fall off the wagon is method injection. If we look at where other languages are heading, they are learning that controllers tend to just be procedural as opposed to object oriented. In Scala, they can even just be static objects.
This convention may only need to apply when the framework is calling project code, but confers some serious savings in boilerplate and even performance by only initializing what's needed for a specific run on the system:

public class MyController
{
    public ActionResponse StoreUser(UserRegistrationService userRegistrationService)
    {
    }

    public ActionResponse ShowUser(UserDirectoryService userDirectoryService)
    {
    }
}

What stands out here is that when StoreUser is called, no instances of UserDirectoryService are obtained (and thus, instantiated) from the container. Contrast this to constructor or property injection which are a package deal. Controllers are just one of a handful of scenarios where method injection from the framework make sense. I certainly wouldn't require that it be available everywhere, but is really awesome to use.

---

Okay, there was a lot here, so I'm going to summarize:

• Everything is resolved from the container.
• Any and all constructors as a result support dependency injection.
• @inject remains the same.
• @inject supports injecting anything the container can resolve - I think it's unrealistic to try and canonicalize certain types as "Services".
• [Inject] is added for properties
• Method injection is only available at certain times and behaves like constructor injection
• [Activate], [FromServices], [FromService], [ActionContext] removed.
• Contexts are scope-bound to the container and are injected when requested, just like everything else.
• Things like ViewData are scope bound to the current request scope and as a result are now injectable too!

It sounds like some of this would require that the default ASP.NET dependency injection system expand a bit, but I see very compelling reasons that will benefit all sides. The community will always have to code to the lowest common denominator, so that bar really ought to be set high out of the box.

[atrauzzi]

@Eilon - If it's okay, I wanted to sound off on some of your responses...

Your first to @michaldudak admits that it's both "confusing" and "with good reason". I'm interested in knowing a bit more as to why. Especially when compared to just a generic injection mechanic.

For your response to @mdekrey, the - living - message here is that we shouldn't have a concept of "DI" vs. "other stuff". By my own experience while learning ASP.NET MVC and when reading scaffold code: These one-off quasi-injections are way more confusing than if you told me "it's just getting injected" with constructor or attribute injection.

I wouldn't argue that most folks stick with defaults, but I also wouldn't rule out putting in strong fundamentals because of that. Up until now, the framework has relied on the base controller because there hasn't been a convenient injection system like what's being offered out of the box with ASP.NET 5. This story stands a chance of changing after release.

[atrauzzi]

Agh, sorry. I think my brain is taking a break today.

[mdekrey]

After having implemented the child scoped-services behavior (see my commit that gets mentioned above), I'm torn. The branch does handle adding services to a child scope for the default container, but I did not completely extend it to Ninject and Autofac. (I wrote tests to show that those are failing, too.) It does not introduce any breaking changes for existing functionality that I could find from the tests, but would allow creating a child scope to, say, register an ActionContext for injection. I think this is a good start for what @glen-84 was making for his point 1 option 2.

However, I completely understand the point @davidfowl and @Eilon are making, and agree; as developers of our own stand-alone projects, when we are building a project that requires advanced features, we can break down that wall and use the DI component that best provides them.

On the other hand, the fact that ASP.NET 5 needs to implement their own property injection and fake a child container using [ActionContext] [ViewContext], etc., indicates to me that the feature set provided is too narrow for the DI abstraction as it stands. Package developers (such as the MVC team) have a different story they need to approach: make it easy for the consuming developers making stand-alone projects. There are many developers and companies who do just this; I would argue that while this isn't "most" developers, it is many developers who will be affected.

Instead of talking about what features to include or not to include in the DI container, I'd actually rather see hooks that we can tie into the DI build process to add these features. Want to add property injection? Add to a theoretical AfterBuild hook, or include this package that does it for you. (I see now that the MVC team built the FromServicesAttribute into model binding. That's pretty slick! I don't think it would go away necessarily.) Want to add child scoped-services? That'd probably be a bit more work, but there could still probably be an additional package depending on your DI implementation that does it for you. Right now, though, I don't see a way that we can do that without relying on a specific underlying system, which breaks the use case for package developers, who will likely need these features the most.

---

@atrauzzi - I agree with most of your points, too, but the hang-up for me on method injection is this: how would those get called? I fully believe that, if we could nail down the syntax, a package could be provided that supplies that functionality already and be DI container independent. Perhaps there's a different thread to discuss this?

[atrauzzi]

My thinking for method injection was that certain dispatch flows in the framework are container aware and would attempt to resolve the parameters prior to calling.

Not opposed to better ideas or improvements, but it's really a convenience to encapsulate at a method level rather than class.

[glen-84]

I see now that the MVC team built the FromServicesAttribute into model binding. That's pretty slick!

See the last paragraph in my comment here.

... but the hang-up for me on method injection is this: how would those get called?

The framework controls the execution of controller methods, so it would just look for [Inject] attributes, and then request those types from the services container. I'm probably forgetting something though.

[jnnwnk]

I am getting errors in intellisens saying type or namespace "ActivateAttribute" is not available in Microsoft.AspNet.Mvc. I never used the attribute. The error refere to my all my layout files and they even appear without content in those files. This started after updating SDK to beta5. The app is running fine, but seems confusing. Has anybody an idea about that?

[Bartmax]

@janna173 I was about to ask the same. In my case is on _ViewImports.cshtml

[developer1998]

@janna173 @Bartmax You can safely ignore them, I believe. It is the Visual Studio error not the framework. The rename did not synch with Visual Studio. You will notice that when you close that view and run run your project no errors and everything works as it should.

[ielcoro]

👍 to constructor injection, resolving everything from the same container, and [Inject] attribute should be there for extremely special cases.

Having two different concepts for handling dependencies is clearly confusing.

[brockallen]

Related -- do we have a way to augment the DI system for per-request dependencies? IOW, something comparable to IHttpContextAccessor but for custom types?

[Garciat]

@brockallen Scoped services, maybe? They are (lazily) instantiated and disposed once per request.

[brockallen]

Right, but MVC itself needs to make the ActionContext in the middle of the request then make it available for injection. Scopes services aren't quite the same since it's configured 'statically'. I'd like a way to inject an instance at runtime for DI later in the request pipeline (like MVC needs to do).

[mdekrey]

I agree, @brockallen. This is called "Child Containers" and is available in most DI containers, but not available for Microsoft.Framework.DependencyInjection. I'm looking to see if there's a way to make an extension...

[herecydev]

@eashi You just linked this thread...

[eashi]

I deleted the comment to remove confusion (I hope I didn't make it worse :P) thanks @herecydev

[NTaylorMullen]

Note: If you're using the latest Mvc packages you may encounter the following errors:

This is due to VS code generating [Activate] for some pieces of Razor code (activate no longer exists). If you run your application you'll see that the errors don't impact anything.

Workaround to avoid errors at design time:

Add the following class/namespace combo anywhere in your project:

namespace Microsoft.AspNet.Mvc
{
    public class ActivateAttribute : Attribute
    {
    }
}

[developer1998]

You also have other issues, declarations in _ViewImports.cshtml are not picked up on design time. So my views are flagging many errors such as:
The name 'someClass' does not exist in the current context
The type or namespace name 'AppSettings' could not be found (are you missing a using directive or an assembly reference?)...

I know this will be resolved, just want to bring it up to attention...

[Eilon]

@developer1998 do you know if there's an issue on this logged in the Razor repo? (It might end up going to the Tooling repo but we could start out in the Razor repo.)

[developer1998]

@Eilon No idea, but I can create it on the Razor repo.

[Duke360]

@atrauzzi +1 plus i'm interested to understeand why certain things are not "service" enough to the ASP.NET team at the point that they require their own DI mechanism, that's not sarcastic, i'm really interested in understand

[BrianVallelunga]

Constructor injection makes the most logical sense from the developer's point of view, but I understand the complexity involved in injecting things that aren't DI services.

If you can't do constructor injection, here are three other options that I just thought up:

1. Rename [Activate] to something like [MvcContext] or [FromMvc] so it's clear that it is only for MVC . Having multiple different attributes for the same action of injecting things seems strange.

2. Use magic. If there are only a few types that this applies to, then just auto-inject them when the public properties exist.

3. Use interfaces. After DI does its thing with the constructor parameters, the framework can see if a class implements an interface and sets the properties as needed.

// Defined in framework
public interface IRequireViewComponentContext
{
    ViewComponentContext ViewComponentContext { get; set; }
}

public class MyViewComponent, IRequireViewComponentContext
{
    public MyViewComponent(IRepository repository)
    {
        Repository = repository;
    }

    public IRespository Repository { get; }

    public ViewComponentContext ViewComponentContext { get; set; }
}

[ThoughtHaven]

If they really, truly can't be constructor injected, then [FromMvc] is the most logical and self-documenting. That's my vote.

[LordZoltan]

I personally think [Activate] is far more preferable to these different attributes - in the absence of constructor injection - I'm sorry but it just looks a horrible mess.

I see the argument against constructor injection, I really do, and I don't really want to have to add loads of constructor arguments for objects that I take for granted in my controllers etc.

So bundle as much of this all up into one constructor argument dependency, and then require your DI implementation to support per-request dependencies. It's already a confirming container - so bit the bullet and make it conform some more.

[glen-84]

@BrianVallelunga @ThoughtHaven,

👎 for [FromMvc]

Finally, it seems to me that [FromService] is inconsistent with the other [From*] attributes, in that all the others bind data from the request, whereas [FromService] simply resolves the object via DI. i.e. The others (I assume) create an instance of the object and bind the properties to data from the request, while [FromService] requests an instance from DI – two different concepts in my mind. For this reason I think that [FromService] could be removed in favour of [Inject].

aspnet/Mvc#2151 (comment)

[NeelBhatt]

+1 for @Inject .. I wrote blog post for @Inject which is here: https://neelbhatt40.wordpress.com/2015/08/14/inject-new-directive-of-mvc/