[Discussion] FromServicesAttribute is no longer applicable on controller or model properties

[pranavkm]

FromServicesAttribute used to be applicable on properties and parameters. The former allowed services to be bound on controller and model properties. However there was a lot of confusion as to where and how this worked.

This change, limits FromServicesAttribute to only work with action arguments. FromServices on controller properties can be replaced by a DI framework that support property injection.

See https://github.com/aspnet/Mvc/issues/3507 for discussion for past discussions for this change.

[dmccaffery]

:+1:

[tuespetre]

Ugh... :-1: :zap:

• Controllers: constructor injection can become tedious/hideous and we shouldn't have to write little 'service grouping' classes just to make our constructors more concise.
• Models: it is so convenient to tack on a [FromServices] property onto a model and then use it for validation (a-la IValidatableObject.) I don't want to have to write a custom validation attribute or model binder thing just to use a certain service within my model's validation logic. Examples:
  • Checking if a username already exists
  • Checking against a value stored in the user's session
  • Checking if the current user is authenticated to determine if a specific field should be required or not

[dmccaffery]

@tuespetre : use a full-blown IoC container like Ninject and you can do whatever you want.

[tuespetre]

I don't want to use a 'full-blown IoC container like Ninject'. That defeats the whole purpose of having dependency injection built in to ASP.NET 5.

The scenarios I mentioned above are so commonplace it's not even worth debating. I am just wondering who complained so much about [FromServices] on properties that the team felt the need to cripple it the way they have.

[Eilon]

@tuespetre the default DI system provided in ASP.NET is not meant to be a feature-rich system.

The decision to remove property support from [FromServices] was made specifically because of customer feedback. We saw that very few people were using it for properties, and that of the few who tried to use were using it incorrectly and it didn't work. This ended up causing many headaches for many people.

We've been working with the community to provide additional features via community-owned DI systems. The DependencyInjection repo has a readme that links to other DI systems that offer additional features and enhancements.

[dmccaffery]

@tuespetre As an FYI: I wasn't one of the ones that complained, but I always felt as though the built-in DI should be just enough to complete the story for ASP.NET internals rather than act as a 'replacement' for the many tried-and-true DI systems out there. They've done just that, which gives us the freedom to use whatever DI system we want -- always a good thing, especially in OSS.

[tuespetre]

Switching to use another DI container would be awesome if I wanted to do all sorts of Mickey Mouse shit like scan unknown assemblies and manipulate a newly-activated instance of a service, but as far as I can tell, switching to another DI container does exactly nothing in relation to binding a service to a model property. All it does is return a different implementation of IServiceContainer from ConfigureServices. In other words, this has nothing to do with DI and everything to do with model-binding, and with the change that has been made here, whether they use another DI container or not, developers who want to bind services to controller and model properties are going to have to plumb it out themselves using the constructs provided by the MVC framework. Feel free to prove me wrong!

It will be mildly annoying to have to put this snippet of plumbing into the root of all or most of my projects...

using System;

[AttributeUsage(AttributeTargets.Property | AttributeTargets.Parameter, AllowMultiple = false, Inherited = true)]
public class FromServicesAttribute : Microsoft.AspNet.Mvc.FromServicesAttribute
{
}

...when the team could have just put a better, more descriptive summary on FromServicesAttribute:

/// <summary>
/// Specifies that a controller action parameter, controller property, or model property should be bound using the request services during model binding.
/// </summary>

I just don't feel like the team considered the very real-world scenarios that I outlined in my previous comments when making this decision. I have commented on the commit that removed the property attribute target to point out where I think a red flag should have gone up.

[singlewind]

I partly agree @tuespetre we write code because like others to use them. If the built-in DI could not meet the common use then it comes waste. However, I don't think current DI need more functionality, because it covers most of needs already.

[NeelBhatt]

:+1: I knew it will happen. All can see strong reaction against [FromService] here:

https://github.com/aspnet/Announcements/issues/28

and here:

https://github.com/aspnet/Mvc/issues/2151

[tuespetre]

I've been reading over these discussions and frankly, I am baffled. We have a few mentions of how [FromServices] has caused a lot of confusion, a lot of headaches, and a lot of posted issues:

@pranavkm: "there was a lot of confusion as to where and how this worked."

@Eilon: "This ended up causing many headaches for many people."

@rynowak: "we've had a number of issues posted by users with confusion around how this feature should be used. There's really been a fairly large amount of feedback"

I can't seem to find any such posted issues regarding confusion about where to use [FromServices], at least not in this repository. Is there some other public place that is officially used to track this kind of feedback? I did find a total of three on StackOverflow:

• http://stackoverflow.com/questions/33535824/asp-net-5-dependency-injection-is-the-fromservices-attribute-only-works-in-a
• http://stackoverflow.com/questions/30759403/dependency-injection-asp-net-5-custom-classes-what-is-correct-way
• http://stackoverflow.com/questions/31324406/how-to-inject-dependency-of-applicationdbcontext-in-repository-mvc6

We also have this notion that there has been an outpouring of feedback from developers about [FromServices]:

@NeelBhatt: "All can see strong reaction against [FromService] here"

@rynowak: "We've heard feedback [...] that [FromServices] is a bit awkward and oddball."

Having looked over the discussion that contains the ten-or-so criticisms about it, there was no mention of its usability or functionality; rather the general sentiment that was expressed was "the naming of it doesn't seem right".

@justinvp: "Personally, I prefer [Inject] over [Activate] and [FromServices]"

@eamodio: "IMO, using "inject" aligns well with Dependency Injection while also expressing more with less confusion than [FromServices]"

@ctolkien: ":+1: on [Inject]"

@bangoker: ":+1: on [Inject] as well"

@atrauzzi: "I agree, [Inject] sounds a lot better than [FromServices]"

@davidroth: ":+1: on [Inject] as it is clear from the name whats happening. [FromServices] sounds very ambiguous"

@nil4: ":+1: for [Inject] rather than [FromServices]"

@evil-shrike, I must single you out for this comment:

@evil-shrike: "I'm afraid that emphasizing "services" contradicts the idea of DI a bit. It'd be ok for "Service Locator" but not for Dependency Injection - as "dependency" isn't always a service."

Throughout the ASP.NET 5 framework, every reference to the dependency injection subsystem refers to "Services": IServiceCollection, ConfigureServices, RequestServices, ApplicationServices, etc. This is not just a matter of the [FromServices] attribute.

And @glen-84, I am sorry but I do feel the need to single you out for this comment:

@glen-84: "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."

If model binding was strictly an abstraction for binding values from an HTTP request, you would be correct, but as far as I can tell, it is not. It is an abstraction for binding values from a binding source, which may happen to be an HTTP request in most cases, but not always.

Every single one of those comments I quoted from #2151 seemed to miss that point entirely, and even though @dougbu and @danroth27 attempted to clarify that this is not a matter of the dependency injection subsystem but rather of the model binding subsystem, their comments seem to have gone unnoticed:

@dougbu: "FYI [FromServices] is named for how that attribute is used elsewhere in MVC model binding"

@danroth27: "[FromService] is a model-binding concept, and allows arbitrary plugging of services on parameters, models, properties"

I don't mean to be angry or rude or anything. I just really feel the need to express my disappointment and concern that it only took roughly ten or so comments on a GitHub discussion -- none of which entailed anything other than purely aesthetic concerns -- and a truthfully small number of posts online for the team to justify the crippling of a very useful feature which would probably be used a lot more than you would think or even be able to reliably measure (looking at you @Eilon):

@Eilon: "We saw that very few people were using it for properties, and that of the few who tried to use were using it incorrectly and it didn't work."

My plea to the MVC team

You have already written the code for the feature and I have demonstrated common, real-world use case scenarios for it. Please revert your decision, add the AttributeTargets.Property back onto the attribute definition, and edit the documentation/summary of the attribute to give it all the clarity it needs to help prevent any confusion surrounding its usage. You could also ensure that all of the official documentation avoids the word "inject" and instead uses the word "bind" when using the attribute, because it is not primarily a dependency injection construct, but a model binding construct.

[anfomin]

I use [FromServices] to bind model properties and it's very useful. And I don't want to use 'full-blown IoC container' for just this feature :-1:

[YaoaY]

:broken_heart:

[Igmat]

I agree with @tuespetre - IMO using this attribute for dependency injection in controllers looks much more better than constructor injection. Code with this feature looks more expressive and readable, because I can omit unnecessary constructor which in most cases needed ONLY for DI.

[atrauzzi]

I'm actually totally confused now. You know what would really be helpful is if a blog post or a topic on announcements was made to describe the feature, what it's intended to do and how it stands currently.

My views on dependency injection might be oriented a little strange to what I think I'm reading from everyone here...

Really, DI should only be used for infrastructure. The fact that I'm hearing about injection happening in models or with models being injected raises huge red flags for me.

Strictly in speaking as a developer using ASP.NET MVC, the first place DI should be triggered - Startup.cs not with standing - is in the controller's constructor and/or the controller's action methods. I think it's easy to see that if DI is allowed everywhere, it just gets out of control and we could run afoul of other competing lifecycles like EF.

Use DI for infrastructure at request time, heck, get rid of all these annotations entirely. Just automatically do DI on controller classes and action methods. You will cover 99% of every scenario and come up with that "bare minimum for a built in DI system".

And like I always do, please see Laravel's DI system which is just so straightforward.

[dmccaffery]

@Igmat : a constructor exists as a concept specifically for the purpose of initializing an object with the necessary dependencies... thats its purpose. Properties as a concept are used to mutate an object after it has been initialized. Using property injection with attributes might seem more readable, but it doesn't fit the lifecycle of object-oriented design. For dumb objects like models, using property initialization is fine, since there are so many possible mutations -- but for something like a controller, where the dependencies are always the same, constructor injection is the right place to do it.

@atrauzzi I'm in agreement with you; adapting behavior via dependency injection is purely about infrastructure (in my world view) ... its core purpose is not to dynamically adapt behavior to solve business problems.

[Igmat]

@dmccaffery I can't agree with you. We are working in OO-language, so all things here is objects by design, but controller is abstraction that has some specific use case. You actually will never use it as object. You won't write things like var ctrl = new MyController(); or ctrl.SomeMethod();. You will never create this object, get it via DI, call its methods or set its properties. Controller have to not contain any business logic. Controller is just a mediator between client code and server code. Its only purpose is to correctly handle requests by calling right services methods, and choose what to send for a client. So controller can be and should be as declarative as possible. We already have some declarative things there like [Route], [Authorize], [HttpGet] etc. Even using MyCoolModel but not ActionResult while describing actions in controllers is a way to more declarative style. And adding additional will just simplify our work. When we write public IMyService MyService { get; set; } or private IMyService _myService; we have already described our intention - to add a dependency. Everything else is redundant for this task in controller. P.S. But I agree that constructor is right place for injection in services. I have never used property injection there. P.P.S. But if we could use property injection for private properties - it would be much more better than constructor injection in most cases.

[ctolkien]

When we write public IMyService MyService { get; set; } or private IMyService _myService; we have already described our intention - to add a dependency

In the case of the property, no you haven't described it as a dependency. That's the issue.

Will add my voice to ctor only injection.

[atrauzzi]

@ctolkien Agreed, properties are ambiguous in meaning as they can be set at any point during an object's lifetime. It is not safe to assume that because something is declared as a property that it is a "dependency".

I wonder -- is it possible that some of the confusion around how people use these things is as a result of the overuse of annotations? Like, what are we really annotating for here?

[tuespetre]

It seems that some folks are still missing a crucial detail: the [FromServices] is not, and never has been, a means of dependency injection. It has nothing to do with the ASP.NET dependency injection system, at all. Using the [FromServices] attribute does not cause the dependency injection system to inject anything onto anything else's properties. It instead tells the model binding system to a bind a property or argument by getting its value from the dependency injection system.

Unless I am mistaken, there is no support right now for parameterized constructors on model classes. If there is, I will stop complaining right now. Otherwise, we need [FromServices] to work on model properties in order to make use of services for validation purposes.

[dmccaffery]

I completely understand that. The effect is; however, the same. Whether the service provider injects the implementation itself, or a 'middleman' uses the service locator anti-pattern to acquire a value and assign it to a property on its behalf, you're still using dependency injection.

Short answer is; using the dependency injection system in .NET for any purpose other than injecting dependencies is promoting it's use beyond its original intent. It wasn't meant to be a fully featured IoC container. It was meant to provide the bare minimum requirements for the framework itself to support composition and separation of concerns through loosely coupled abstractions.

If you want to use IoC to effect behavior rather than infrastructure, then use a fully featured IoC container that allows for this. .NET is fully extensible, including its IoC implementation.

I'm not arguing for or against these different patterns; I'm only arguing against trying to make the built-in dependency injection system attempt to go beyond its means. If MS were to bring this attribute back, they would also need to negate its misuse, which would require the DI to be much more involved -- something that 3rd parties have already done.

[tuespetre]

@dmccaffery:

If you want to use IoC to effect behavior rather than infrastructure, then use a fully featured IoC container that allows for this. .NET is fully extensible, including its IoC implementation.

If you want to promote 'using a fully featured IOC container' to fill the gap created by disabling the use of [FromServices] on model properties, then please explain exactly how another IOC container could be used to do so. Post a code sample. Publish a Gist.

[roydukkey]

But if we could use property injection for private properties - it would be much more better than constructor injection in most cases.

So I must admit, I don't like [FromServices] now or before. The above quote was the original reason. Take the following example:

public abstract class RavenController : Microsoft.AspNet.Mvc.Controller
{
    [FromServices]
    public IDocumentStore DocumentStore { get; set; }
    public IDocumentSession Session { get; set; }

    public override void OnActionExecuting(ActionExecutingContext context)
    {
        Session = DocumentStore.OpenSession();
        base.OnActionExecuting(context);
    }

    public override void OnActionExecuted(ActionExecutedContext context)
    {
        using (Session) {
            if (Session != null && context.Exception == null) {
                Session.SaveChanges();
            }
        }
        base.OnActionExecuted(context);
    }
}

public class PageController : RavenController 
{
    public IActionResult Index()
    {
        Employee employee = Session.Load<Employee>(employeeId);
        ...
    }
}

The proerty, DocumentStore, really should be private, not public, but that is how [FromServices] worked. The alternative was/now is more hideous:

public abstract class RavenController : Microsoft.AspNet.Mvc.Controller
{
    private IDocumentStore _documentStore { get; set; }
    public IDocumentSession Session { get; set; }

    public Controller([FromServices] IDocumentStore documentStore)
    {
        _documentStore = documentStore;
    }

    public override void OnActionExecuting(ActionExecutingContext context)
    {
        Session = _documentStore.OpenSession();
        base.OnActionExecuting(context);
    }

    public override void OnActionExecuted(ActionExecutedContext context)
    {
        using (Session) {
            if (Session != null && context.Exception == null) {
                Session.SaveChanges();
            }
        }
        base.OnActionExecuted(context);
    }
}

public class PageController : RavenController 
{
    public PageController(IDocumentStore documentStore) : base(documentStore) { }

    public IActionResult Index()
    {
        Employee employee = Session.Load<Employee>(employeeId);
        ...
    }
}

Now the constructor has to be redefine for every child controller. And, I still have access to documentStore in the constructor when it really should be private.

Now, I'm new to MVC as I've always used Web Forms, and throughout the development of vNext I've been keeping a learning project update date with changes and fixing breaking changes as they occur, so that when MVC 6 releases I might move my Web Forms project to MVC. Thus, I might not have a proper idea of DI, but it seems that what I'm trying to do with RavenDB is acceptable logic under DI. It would be much cleaner to have [FromServices] work for private properties.

Additionally, I thought I read somewhere that [FromServices] was only acceptable for one parameter of constructors, and as a consequence [FromServices] properties should be consider in cases where multiple DI object are needed. If this is still true, [FromServices] feature is hardly usable.

[dmccaffery]

Make your IDocumentStore a private read only field; it doesn't need to be public, but you would need to register the IDocumentStore or factory to the services collection for DI to work. You would also need include IDocumentStore as a constructor argument.

The argument against using an attribute to set a private field is related to performance; it's expensive to reflect a type to find all fields, property methods, and constructors looking for things to inject.

Constructor injection follows object-oriented design principles; and is the minimal amount of extra work required to enable injection.

[roydukkey]

Okay. I understand why private fields isn't implemented for [FromService]. But, I still don't understand how to extend a class without making an empty copy of the parents constructor as the second example shows above. Is this expected? This was what I was trying to avoid by using [FromService] on a property.

[tuespetre]

Constructor injection follows object-oriented design principles; and is the minimal amount of extra work required to enable injection.

Which is fine for controllers, but if you want to keep all of your model validation logic together and be able to take advantage of automatic validation during model binding, we need one of two things:

• Support for parameterized constructors in model classes
• The [FromServices] attribute

It's been three weeks now; I know it's probably been busy for everybody but would an MVC team member have any updates to share with us? I feel that the "using services from the service container to implement model validation logic" story is very important and I would just like to know whether, at the very least, some unspecified solution to this story is in the plans for MVC 6.

[atrauzzi]

@tuespetre In your suggestion, what would you be using the parameterized constructors in the model for?

[tuespetre]

@atrauzzi:

https://github.com/aspnet/Mvc/issues/3578#issuecomment-162257937

in order to make use of services for validation purposes.

https://github.com/aspnet/Mvc/issues/3578#issuecomment-163257863

if you want to keep all of your model validation logic together and be able to take advantage of automatic validation during model binding

https://github.com/aspnet/Mvc/issues/3578#issuecomment-157825813

Examples:

• Checking if a username already exists
• Checking against a value stored in the user's session
• Checking if the current user is authenticated to determine if a specific field should be required or not

Pseudo-code example of what I am doing now with rc1:

public class SignupFormViewModel : IValidatableObject
{
    [FromServices]
    public UserSession Session { get; set; }

    [FromServices]
    public IHttpContextAccessor HttpContextAccessor { get; set; }

    // Lots of properties I'm omitting for brevity

    public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
    {
        // Lots of simple validation rules I'm omitting for brevity

        if (!HttpContextAccessor.HttpContext.User.Identity.IsAuthenticated)
        {
            if (SomeConditionFails())
            {
                yield return new ValidationResult(SomeMessage, new [] { nameof(SomeProperty) });
            }
            else if (AnotherConditionFails())
            {
                yield return new ValidationResult(AnotherMessage, new [] { nameof(AnotherProperty) });
            }
        }

        if (Session.SomeConditionIsPresent() && DifferentConditionFails())
        {
            yield return new ValidationResult(DifferentMessage, new [] { nameof(DifferentProperty) });
        }
    }
}

The beauty of [FromServices] (or possible support for injecting dependencies into parameterized constructors in models) is that you have all of the validation rules for an object in one place and you don't have to use the service locator anti-pattern to get what you need. I'm not even sure how to use that anti-pattern in MVC6 since the service container is not made available as a static reference, as far as I can tell.

Nobody in any of the discussions surrounding [FromServices] who has stated "Just use another DI container" has bothered to explain how that would help or how it applies to this situation, and it seems that you are the first person willing to consider the concerns that I am voicing, so thank you for that. I look forward to seeing what you have to say.

[dmccaffery]

@tuespetre

Firstly, I did leave a comment of how it could be done; you could write your own model binder, or validate (like you did) and resolve types using the anti-pattern you mentioned. You can even do it without a separate DI container by saving off the service provider during Startup to a static field to enable the anti-pattern. You can also use filters to effect behavior based on the current state of the system, which is probably how this would have been done in the past.

Secondly, It's still a bad idea. While your specific use case seems reasonable (and it is -- it's the use case MS was trying to support, I believe), enabling dependency injection on the model during model binding is dangerous (out of the box) for several reasons:

1) I can see use cases where people are going to use [FromServices] in model binding to extend the model -- like reading some additional info from session, or a cache, like this example which already cropped up a year ago:

http://shazwazza.com/post/model-binding-with-fromservices-in-aspnet-5/

This is a horrible idea, and I'm sure that everyone will agree with me on this, at least.

2) If the injection fails, or any code within the injected types fail, an exception is thrown that cannot be caught by user code, which means difficult, hard to find debugging scenarios.

3) Fat model, skinny controller was not embraced by Microsoft in ASP.NET MVC, for better or worse. Aside from validators, the design of the system reinforces the idea that models are more like DTOs for a view -- not fully-baked, self-aware, self-managing objects.

[dmccaffery]

@tuespetre: Oh crap: I stand corrected. I wrote a comment on how it could be done; of that, I am sure. For whatever reason, I don't think it persisted for whatever reason. It was yesterday, at some point. Oops. My sincere apologies.

[tuespetre]

@dmccaffery

you could [..] resolve types using the anti-pattern you mentioned. You can even do it without a separate DI container by saving off the service provider during Startup to a static field to enable the anti-pattern.

Except then you lose the concept of request-scoped services, let alone the ability to access the HttpContext itself.

1) I can see use cases where people are going to use [FromServices] in model binding to extend the model -- like reading some additional info from session, or a cache, like this example which already cropped up a year ago:

http://shazwazza.com/post/model-binding-with-fromservices-in-aspnet-5/

This is a horrible idea, and I'm sure that everyone will agree with me on this, at least.

Yes, I can agree with that, but you can do a whole bunch of horrible things with a lot of other things in a lot of other ways. I could write an application that uses Entity Framework to build a 'meta', proprietary, crappy data storage system within a relational database. I could write controller actions that have no logic within them and then write filters that return all the results. I could pass everything around via HttpContext.Items and implement my own vomit-inducing validation system inside of it.

That's no excuse for taking something away that has perfectly valid and sensible use cases.

2) If the injection fails, or any code within the injected types fail, an exception is thrown that cannot be caught by user code, which means difficult, hard to find debugging scenarios.

That's going to be the case when injecting services into controllers as well, or using the [FromServices] attribute on an action method parameter, or running any custom validation code via attributes or IValidatableObject. That's what testing is for; it's just the nature of the beast!

3) Fat model, skinny controller was not embraced by Microsoft in ASP.NET MVC, for better or worse. Aside from validators, the design of the system reinforces the idea that models are more like DTOs for a view -- not fully-baked, self-aware, self-managing objects.

I fully acknowledge that; I am speaking only of the binding and validation aspects of the framework.

[atrauzzi]

@tuespetre What @dmccaffery said is more or less where I was heading with my question.

What happens when you're running CLI outside of a request and you try to use a model that depends on requests?

[blakepell]

Late to the party, I see this has already been changed for RC2. I was happy when I found this and then disappointed when I found this thread today seeing it's going away.

In my case, I had services being injected via a constructor in a base controller. Having to pass the same constructor around repeatedly was cumbersome and a little annoying to refactor when a change needed to be made to the base class causing all inheriting constructors to need to be changed. I used the FromServices to slim the constructor but provide those services via properties that wouldn't break when I added a new one (maybe that's a poor use but it was easy to provide those services without busting everything down the line).

I assume I can group those services into a class like a service manager where they're injected and then use that in the constructor for the same result but it seems like a needless step after having used this. That's just my two cents.

[dmccaffery]

@blakepell

I feel your pain, and can only offer this as a solution:

Use a full-blown IoC container, such as Autofac; it can do what you need it to do. The built in dependency resolver is designed solely for the purpose of injecting types necessary for supporting the framework; it wasn't meant to be a replacement for the other, quite good, and fully-featured IoC containers that are available; primarily because there are different schools of thought as to how a container should register types, etc, and Microsoft seemingly wants to continue to enable developers to make our own choices when it comes to pervasive design patterns.

[blakepell]

@dmccaffery That's fair, I'll take a look at Autofac and also Ninject, thanks for the suggestions, it is appreciated. The more this evening I think about a service manager injected through the constructor the more I'm warming up to it, it does fulfill the need of being able to update without breaking inheritance. Thanks for the response also.

[dmccaffery]

Ninject isn't supportive of DNX, I don't believe -- it's somewhat a dead project; but StructureMap is there, as is AutoFac. I realized I mentioned Ninject at the head of this thread, but that was me just throwing out an example, not a recommendation. All IoC (and things in general) are left to personal preference.

[DannyvanderKraan]

I am with atrauzzi, dmccaffery (and so on). Whoever liked (and needed) the FromService attribute should read Mark Seemann's "Dependency Injection in .NET". IMHO is should be 90% constructor injection, 9% method injection and 1% property injection. Why? Anti patterns. Decay of architecture. Etc. Not going to explain in 1 comment. See reference to suggested book.

[atrauzzi]

I'm mostly okay with the removal (I think). But I don't like the suggestion that people can plug their own DI in becoming the standard response. It's getting a bit stale and ASP.NET core isn't even out yet.

The MVC/routing dispatcher should support method injection on controller actions. This would ensure peoples' controller constructors don't get too fat.

Otherwise, constructor injection is really what people should be using. Again, as always, I direct people to look at how laravel's injection is made available. It's quite effective.

[DannyvanderKraan]

@atrauzzi IMHO the objections against constructor injection are because of breaking SRP (as said before decay of architecture). Around 3 injections via constructor as a rule of thumb is fine. If it is starting to get more and more it is time to check the cohesion.

[atrauzzi]

@DannyvanderKraan That definitely makes sense and I agree with that philosophy in the absence of method injection. It's 100% common sense.

There's sadly a lot of dissonance that arises out of the terminology. Specifically controller and action. In the absence of method injection, a controller is a cohesive group of actions - as you said.

When method injection is available, the notion of a controller almost completely boils off. It's just a language construct and serves more for organization of code. It becomes more about the actions which can specify different dependencies without stepping on each other's toes.

[davidames]

I found this thread way too late as I have just found that [FromServices] is missing in RC2 and I'm a little disappointed.

Like @tuespetre we do validation in Models using IValidatableObject when we need to validate using dependencies outside of the model itself. (Duplicate checking would be a good example).

We like to keep this sort of validation outside of the Controller, because the pattern of if (!ModelState.IsValid) return View(vm) provides a nice separation of concerns & thinner controllers.

The DataAnnotations attributes also provide a nice separation of concerns, in that for each model, we know exactly where the validation logic, or validation orchestration logic if you are using more of a DDD pattern is. EG, [Required] is validation logic so is the iValidableObject.IsValid.

I think it comes down the fact that you have to have the orchestration logic for validation somewhere, and you either have fatter controllers or fatter models. It's up to us as developers to choose our poison based on our circumstances. No one way is outright good and evil, right or wrong.

Also like @tuespetre, I don't really care how the framework injects dependencies into Models/Attributes (constructor or attribute or even service location via validationContext.ServiceContainer.GetService()) but the point is that they should provide that capability.

Saying "just use a full blown IoC container" as the answer does not really help as you still need custom model binders & attribute factories, and that code is tricky to get right. Probably trickier than using [FromServices]

dave

[tuespetre]

@davidames you can now use the ValidationContext that comes into the Validate method of IValidatableObject to obtain services for use during validation.

On May 22, 2016, at 7:53 PM, davidames notifications@github.com wrote:

I found this thread way too late as I have just found that [FromServices] is missing in RC2 and I'm a little disappointed.

Like @tuespetre we do validation in Models using IValidatableObject when we need to validate using dependencies outside of the model itself. (Duplicate checking would be a good example).

We like to keep this sort of validation outside of the Controller, because the pattern of if (!ModelState.IsValid) return View(vm) provides a nice separation of concerns & thinner controllers.

The DataAnnotations attributes also provide a nice separation of concerns, in that for each model, we know exactly where the validation logic, or validation orchestration logic if you are using more of a DDD pattern is. EG, [Required] is validation logic so is the iValidableObject.IsValid.

I think it comes down the fact that you have to have the orchestration logic for validation somewhere, and you either have fatter controllers or fatter models. It's up to us as developers to choose our poison based on our circumstances. No one way is outright good and evil, right or wrong.

Also like @tuespetre, I don't really care how the framework injects dependencies into Models/Attributes (constructor or attribute or even service location via validationContext.ServiceContainer.GetService()) but the point is that they should provide that capability.

Saying "just use a full blown IoC container" as the answer does not really help as you still need custom model binders & attribute factories, and that code is tricky to get right. Probably trickier than using [FromServices]

dave

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub

[razzemans]

Also sad to see this go. We were using the [FromServices] attribute for the exact same reason as @blakepell. Having to plug a custom DI framework only to accommodate this scenario is a waste of time imho, and we would revert back to ctor injection. But again, having to add a constructor 10 times in our controllers just for the sake of passing a service to our base controller's constructor through DI is way more ugly than injecting something through a property.

[maxisam]

ah... so if enough people come up here and complain about DI framework confusing them, are we going to remove DI from .net then ? Believe me, I know a lot of old style programmers hating DI, I can tell them this "good news". When someone say something confuse him, I think we should figure out why instead of removing it. And honestly, if this confuses them, why do they need to use this way ? It is an optional solution not a mandatory one. It is like you can either walk on ice or slide on it. And someone think sliding is confusing, or don't know how to slide, so we forbid people to slide. Anyway, just feeling frustrating about how many code I need to change for this breaking change.

[singlewind]

@maxisam DI is good pattern to save a lot of time and prevent "old style programmers" to write unmaintainable code. Writing code is not just for features and function working. It is also for read by others and maintainability.

[blakepell]

@davidames @razzemans I extracted this class and it appears to allow FromServices to work in RC2 from initial testing (although I should verify I was using all RC2 bits and didn't re-reference an old piece for this). I'm guessing probably not the best thing to do but, thought I'd share what I found).

[AttributeUsage(AttributeTargets.Property | AttributeTargets.Parameter, AllowMultiple = false, Inherited = true)]
public class PropertyFromServicesAttribute : Microsoft.AspNet.Mvc.FromServicesAttribute
{

}

`

[razzemans]

@blakepell Yes thanks, we ended up using that as well, as described by @tuespetre in this thread. In my opinion, it's a good solution and I think it's exactly what the [FromServices] used to be in RC1.

[Kizmar]

I am also disappointed and baffled that this change was made. 👎

[tariqalardah]

Now if I am implementing a CustomAttribute that needs services in it. How to inject services inside the Attribute?

[Eilon]

@tariqalardah injecting arbitrary services into an arbitrary attribute was never supported and I don't think it ever can be - construction of attributes is done by .NET itself, which isn't aware of DI or services.

Is this an arbitrary attribute or is it an MVC filter attribute?

[tuespetre]

@tariqalardah to add on what @Eilon said, if you are making a custom filter attribute, you probably want your attribute to implement IFilterFactory, which can then return your custom instance of IFilterMetadata from the service container (or use the service container to obtain the dependencies needed by your filter.)