JasperFx / lamar

Fast Inversion of Control Tool and Successor to StructureMap
https://jasperfx.github.io/lamar
MIT License
566 stars 117 forks source link

Setter Injection not working (Core 3.0) #220

Closed stevestokes closed 4 years ago

stevestokes commented 4 years ago

Create a default .Net core 3.0 web app

Nuget Lamar 4.0.0 Nuget Lamar.Microsoft.DependencyInjection 4.0.2

Program:

    public class Program
    {
        public static void Main(string[] args)
        {
            CreateHostBuilder(args).Build().Run();
        }

        public static IHostBuilder CreateHostBuilder(string[] args) =>
            Host.CreateDefaultBuilder(args)
            .UseLamar()
            .ConfigureWebHostDefaults(webBuilder =>
            {
                    webBuilder.UseStartup<Startup>();
            });
    }

Startup:

        public void ConfigureContainer(ServiceRegistry services)
        {
            services.Scan(scanner =>
            {
                scanner.TheCallingAssembly();
                scanner.WithDefaultConventions();
                scanner.SingleImplementationsOfInterface();
            });

            services.Policies.SetAllProperties(y => y.OfType<ISetter>());
        }

        public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
        {
            // debug
            var container = (IContainer)app.ApplicationServices;
            var plan = container.Model.For<WeatherForecastController>().Default.DescribeBuildPlan();
            Console.WriteLine(plan); // this shows both inline AND setter in the build plan

            app.UseRouting();

            app.UseAuthorization();

            app.UseEndpoints(endpoints =>
            {
                endpoints.MapControllers();
            });
        }

Some POCO + Interface:

    public interface ISetter
    {

    }

    public class Setter : ISetter
    {

    }

Controller:

    [ApiController]
    [Route("[controller]")]
    public class WeatherForecastController : ControllerBase
    {
        [SetterProperty] // doesn't help
        public ISetter setter { get; set; } // always null, with, or without attribute

        ...derp...

        public WeatherForecastController(ISetter _setter) // this works
        {

        }
    }

I've tried every flavor of setter / property injection, nothing seems to work. I had this working in StructureMap though.

Side note: when using a public abstract class without a constructor, the build plan throws an error. I thought this was my original problem but when I did a vanilla setter injection test as described above, it seems to just be broken.

Example:

    public abstract class BaseController : ControllerBase
    {
        // even when setup properly these are always null. 
        public IMediator _mediator { get; set; }
        public IHubContext<LogHub> _logHubContext { get; set; }
        public IConfiguration _config { get; set; }
jeremydmiller commented 4 years ago

@stevestokes,

Just for fun, try WhatDoIHave() on your application. I think you're going to see that ISetter has no registrations.

Try replacing:

            services.Scan(scanner =>
            {
                scanner.TheCallingAssembly();
                scanner.WithDefaultConventions();
                scanner.SingleImplementationsOfInterface();
            });

with some kind of explicit specification of the application assembly:

            services.Scan(scanner =>
            {
                scanner.AssemblyContainingType<ISetter>();
                scanner.WithDefaultConventions();
                scanner.SingleImplementationsOfInterface();
            });

I'll pursue the type scanning issue, but the setter injection mechanics are all fine when I set up a test to recreate this.

jeremydmiller commented 4 years ago

@stevestokes Hey, and check your type scanning diagnostics too. If there is no ISetter implementation registered, Lamar is going to ignore all the setter injection rules

stevestokes commented 4 years ago

Hey @jeremydmiller I added:

            scanner.AssemblyContainingType<ISetter>();
            scanner.Assembly(typeof(Program).Assembly);
            scanner.LookForRegistries();

...

            var container = new Container(_ =>
            {
                _.For<ISetter>().Use<Setter>();
            });

            services.IncludeRegistry<DerpRegistry>();

            services.For<ISetter>().Use<Setter>();

This:

var whatDidIScan = container.WhatDidIScan();

outputs:

All Scanners
================================================================

Assemblies
----------
* WebApplication1, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null
* WebApplication1, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null

Conventions
--------
* Default I[Name]/[Name] registration convention
* Register any single implementation of any interface against that interface
* Lamar.Scanning.Conventions.FindRegistriesScanner

No problems were encountered in exporting types from Assemblies

Still null on setter: image

Not null on Ctor: image

I've pushed my project so you can see: https://github.com/stevestokes/lamar-setter-test

stevestokes commented 4 years ago

Oh and @jeremydmiller I updated to 4.1.0

jeremydmiller commented 4 years ago

@stevestokes Did you check the WhatDoIHave() registrations to see that ISetter exists?

jeremydmiller commented 4 years ago

And there's nothing in that sample above that would opt into setter injection. Is there any real reason you have to have setter injection? I traditionally recommend against it.

stevestokes commented 4 years ago

@jeremydmiller

And there's nothing in that sample above that would opt into setter injection

Did I implement it improperly? I have [SetterProperty] within the WatherController class and all the registrations that I think I need.

I cut out a lot above and below, but WhatDoIHave does include ISetter

IServiceScopeFactory                                                          Microsoft.Extensions.DependencyInjection             Singleton     Current IServiceScopeFactory                                                    IServiceScopeFactory     
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
ISetter                                                                       WebApplication1                                      Transient     new Setter()                                                                    setter1                  
                                                                                                                                   Transient     new Setter()                                                                    setter2                  
                                                                                                                                   Transient     new Setter()                                                                    setter3                  
                                                                                                                                   Transient     new Setter()                                                                    setter4                  
                                                                                                                                   Transient     new Setter()                                                                    setter5                  
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
IStartupFilter                                                                Microsoft.AspNetCore.Hosting                         Transient     new HostFilteringStartupFilter()                                                hostFilteringStartupFi...
                                                                                                                                   Singleton     User Supplied Object                                                            IISServerSetupFilter     
                                                                                                                                   Singleton     new MiddlewareFilterBuilderStartupFilter()                                      middlewareFilterBuilde...

I did read that you recommend not using it, however, I have a use case where it works very well for me. I use a Mediatr CQRS pattern and each Handler inherits a common base class where those setter implementations are set, then used throughout the application.

Example:

    public class SomeHandlers
    {
        public class GetByIDHandler : DbContextHandler, IRequestHandler<ImportMessages.GetByID, Import>
        {
            public async Task<Import> Handle(ImportMessages.GetByID message, CancellationToken cancellationToken)
            {
                await _mediator.Send(new SomeOtherMessageOrEvent());

                return await _db.Imports.FindAsync(message.ID);
            }
        }
    }

Then the DbContextHandler base class looks like:

public abstract class DbContextHandler
    {
        public IMediator _mediator { get; set; }
        public DbContext _db { get; set; }
...
+other stuff
     }

This allows me to call DB & Mediatr operations within my business layer. I also do not have to setup Ctor's in each handler class (note: there are hundreds)

jeremydmiller commented 4 years ago

It's always MediatR's fault. Look, I'm not seeing anything wrong with the Lamar internals here, and the setter injection features are well covered by tests that are happily passing. I can take your sample app for a spin some time next week and see if I spot anything.

stevestokes commented 4 years ago

I would appreciate you taking a look. The example program I've uploaded does not include anything with mediatr, so I don't think this is a mediatr thing.

jeremydmiller commented 4 years ago

Not directly MediatR, but IMO, you've added quite a bit of complexity (inheritance + generic types) explicitly because you are using MediatR. It's a massive pet peeve of mine how often I've had to help folks do complicated things with Lamar or StructureMap because of their MediatR usage.

stevestokes commented 4 years ago

Ah, I hear you. I guess I'm not your avg Joe ;) I was able to use this pattern easily with SM for years (I've used SM for 6+yrs). So I was surprised it wasn't working with a simple poco I/Setter implementation. While MediatR has caused you complexity their solution is pretty nice. Imo event driven code is something I don't think will ever go away even if it's not mediatr. To me it's a natural pattern that'd emerge based on the need to decouple everything. I'll be honest I was excited to see Lamar as I feel like SM was a superior IoC framework compared to others. If I have time I'll take a peek at the Lamar code as well. Time has become more valuable lately vs doing cool projects like this ;)

jeremydmiller commented 4 years ago

"I guess I'm not your avg Joe ;)" -- then I think I'm going to let you take this yourself. I don't see any issues with the setter injection support inside of Lamar and I think this is going to be due to something specific about your code.

stevestokes commented 4 years ago

So I did some investigation. I could use a breadcrumb, you may know where to look. I can see that Jasper is creating the correct code to inject the setter, I get this from DescribeBuildPlan() specific to WeatherForecastController:

using System.Threading.Tasks;

namespace Jasper.Generated
{
    // START: WebApplication1_Controllers_WeatherForecastController_weatherForecastController
    public class WebApplication1_Controllers_WeatherForecastController_weatherForecastController : Lamar.IoC.Resolvers.TransientResolver<WebApplication1.Controllers.WeatherForecastController>
    {

        public override WebApplication1.Controllers.WeatherForecastController Build(Lamar.IoC.Scope scope)
        {
            var setter3 = new WebApplication1.Setter();
            return new WebApplication1.Controllers.WeatherForecastController(){APublicSetterProperty = setter3};
        }

    }

    // END: WebApplication1_Controllers_WeatherForecastController_weatherForecastController

}

WhatDidIScan() includes my only assembly and WhatDoIHave() does show ISetter. All of this is looking good.

I did a direct test on the container in Startup.cs, testing as so: var controller = container.GetInstance<WeatherForecastController>(); This worked as expected, the public property APublicSetterProperty was set and not null.

I noticed when debugging the Lamar code when I call DescribeBuildPlan() or GetInstance() or GetService(Type serviceType)

The code will call down into ContructorInstance.cs and into the findSetters() method:

private void findSetters(ServiceGraph services)
        {
            foreach (var property in ImplementationType.GetProperties().Where(x => x.CanWrite && x.SetMethod.IsPublic))
            {
                var instance = findInlineDependency(property.Name, property.PropertyType);
                if (instance == null && services.ShouldBeSet(property))
                {
                    instance = services.FindDefault(property.PropertyType);
                }

                if (instance != null) _setters.Add(new InjectedSetter(property, instance));
            }

            foreach (var setter in _setters)
            {
                setter.Instance.CreatePlan(services);
            }
        }

This is what I would expect. However, when I comment out my calls to DescribeBuildPlan, GetInstance and GetService it looks like whatever is resolving the MVC Controller WeatherForecastController it is not following the same or similar code paths within IoC\Scope.cs. It looks like the bootstrapping to the Asp Core dependency resolver is not calling into Lamar to instance the MVC Controller? It looks like most of this happens in IoC\Scope.cs, but I am not able to locate where Asp Core calls into Lamar to instance Controller classes, do you know where that entry point is?

Side note, I may be wrong on some of this as I am just learning the code.

stevestokes commented 4 years ago

@jeremydmiller I figured it out. Definitely not a Lamar issue, just an MVC setup issue. You may want to add this to your asp.net Core 3.x docs

services.AddMvc().AddControllersAsServices();

For some reason adding AddControllersAsServices(); fixes it.