Enable dependency injection for server-side code invoked by the data portal

[dazinator]

Apologies for the length of this :-)

I'd like to describe some issues / challenges I have had to work though recently for an asp.net core application. I think there is some potential here for adding some support to CSLA to help enable DI in asp.net core to flow through CSLA scenarios. This could be in the form of adding some support classes, I will describe below.

First a brief overview of my setup:

I have an asp.net core web application, which also hosts a hangfire job service (runs jobs on background threads within the same AppDomain).

My aims:

1. Configure all my services for DI in one place - startup.cs. Use whatever container I like as long as it supports the IServiceProvider adaption (many do).
2. Use DbContextManager in my CSLA DataPortal_XYZ methods.
3. DbContextManager should resolve the DbContext instance that is in scope. More on scopes below. This could be Application scope, Request scope or some arbitrary scope.
4. Use ObjectFactories - and be able to resolve them from current scope.

The reason I say "current scope" is because the current scope can vary depending on where your CSLA usage is:

1. Application level - these services (IServiceProvider) are all configured on startup. This is the default scope if no other scope is overriding it.
2. Request level - at the start of a request, ASP.NET creates a new "scope" for the request. It dumps this IServiceProvider in HttpContext.RequestServices.
3. Arbitrary level - Any time something creates a new service scope - this scope should be able to be used as the "current" scope. For example, hangfire creates it's own ServiceScope for each job executing on a background thread.

Now consider a couple of CSLA extension points that potentially need to resolve dependencies. Here are the ones that were important to me:

1. IObjectFactoryLoader - This creates the ObjectFactory instances. For DI purposes this needs to resolve the factories from the "current scope". If it doesn't respect the current scope, you get bugs. For instance shared instances of DbContexts being used accross multiple factories. You see this in the background job scenario.
2. DbContextManager<C> - When not using ObjectFactories, I often just use DbContextManager<C> in the DataPortal_XYZ methods. However it should resolve the DbContext instance from the current scope.

The problem I have had to work through then is this issue of Current Scope. There is no method (to my knowledge) of just arbitrarily detecting what is the "current scope" - i.e you can't just do this ServiceScope.Current.

Consider the following code, imagine both methods are executing at the same time, on different threads:

public void RunConcurrentlyOnThreadOne()
{
   using (var scopeOne = ServiceScopeFactory.Create())
   {
        var dbContextOne = scopeOne.ServiceProvider.GetRequiredService<MyDbContext>();
        var cslaObject = Customer.Get(1);
        // CSLA (DbContextManager or ObjectFactory) should have both used the same dbContextOne instance.
   }
}

public void RunConcurrentlyOnThreadTwo()
{
   using (var scopeTwo = ServiceScopeFactory.Create())
   {
        var dbContextTwo = scopeTwo.ServiceProvider.GetRequiredService<MyDbContext>();
        var cslaObject = Customer.Get(1);
        // CSLA (DbContextManager or ObjectFactory) should have both used the same dbContextTwo instance.
   }
}

There is no way to write an ObjectFactoryLoader or DbContextManager that automagically discovers these arbitrary scopes. And so my conclusion is, CSLA (or more precisely - the hooks I am talking about like DbContextManager and IObjectFactoryLoader`) have to be told about the current scope at the time of usage.

The current implementation of DbContextManager for EF7 / Core will always create a new instance of DbContext which it then disposes of on the final deref(). My need is for to use the instance within the current scope. This may mean one is created, or it may mean an existing instance is re-used - but its the current scope that dictates the lifetime.

This eventually led to me implementing some additional helper classes to handle this concept of scope with CSLA in asp.net core scenarios:

  
    public static class ServiceProviderCslaExtensions
    {
      
        public static void SetDefaultServiceProvider(this IServiceProvider serviceProvider)
        {
            global::Csla.ApplicationContext.GlobalContext.SetServiceProvider(serviceProvider);
        }

        public static void SetCurrentScopeServiceProvider(this IServiceProvider serviceProvider)
        {
            global::Csla.ApplicationContext.LocalContext.SetServiceProvider(serviceProvider);
        }

        /// <summary>
        /// Returns the local csla service provider if available, but falls back to the global csla service provider if not. Also checks for an active HttpRequest
        /// and uses HttpContext.RequestServices if found.
        /// </summary>
        /// <returns></returns>
        public static IServiceProvider GetServiceProviderForCurrentScope()
        {
            // check local first, then fallback to global.
            IServiceProvider sp = null;
            sp = global::Csla.ApplicationContext.LocalContext.GetServiceProvider();
            if (sp == null)
            {
                sp = global::Csla.ApplicationContext.GlobalContext.GetServiceProvider();
            }
            return sp;
        }      

       /// <summary>
       /// Anti pattern necessary to achieve DI in some places in CSLA framework.
      /// </summary>
        public static TService Locate<TService>()
        {
            var sp = GetServiceProviderForCurrentScope();           
            var implementation = sp.GetService<TService>();
            return implementation;
        }
    }

Basically:

1. The Application level IServiceProvider get's stored in GlobalContext - by calling SetDefaultServiceProvider.
2. The IServiceProvider for whatever the "current scope" is, get's stored in LocalContext and CSLA will use that if it is present, otherwise it will fall back to the default one.
3. In asp.net core web applications (or OWIN) I take the approach of adding Middleware that runs on each request, and calls SetCurrentScopeServiceProvider with the IServiceProvider that is created for current request scope.

I made a tweak to the existing CslaBootstrap class to set the default service provider:

    /// <summary>
    /// A marker class, registered as a singleton - services can take a dependency on this to ensure CSLA has been bootsrapped.
    /// </summary>
    public class CslaBoostrap
    {
        public CslaBoostrap(CslaOptions options, IServiceProvider serviceProvider)
        {           
            Csla.ApplicationContext.WebContextManager = options.WebContextManager ?? new HttpContextAccessorContextMananger(serviceProvider);
            Csla.ApplicationContext.ContextManager = new ApplicationContextManager();
            serviceProvider.SetDefaultServiceProvider();
            Csla.Server.FactoryDataPortal.FactoryLoader = options.ObjectFactoryLoader;
        }
    }

To get this working with Hangfire background jobs, after hangfire creates its scope for the job, I do this:

scope.ServiceProvider.SetCurrentScopeServiceProvider(); // extension method puts service provider into csla local context.

My IObjectFactoryLoader implementation looks like this:

 public class ServiceProviderObjectFactoryLoader : IObjectFactoryLoader
    {
    
        private readonly ConcurrentDictionary<string, Type> _cachedTypes;
        private readonly Func<IServiceProvider> _serviceProviderFactory;

        public ServiceProviderObjectFactoryLoader(Func<IServiceProvider> serviceProviderFactory)
        {
            _serviceProviderFactory = serviceProviderFactory ?? throw new ArgumentNullException(nameof(serviceProviderFactory));
            _cachedTypes = new ConcurrentDictionary<string, Type>();
        }
      

        public object GetFactory(string factoryName)
        {
            Type type = GetFactoryType(factoryName);
            try
            {
                var sp = _serviceProviderFactory();                               
                var implementation = sp.GetRequiredService(type);
                return implementation;
            }
            catch (InvalidOperationException ex)
            {
                throw new ObjectFactoryNotRegisteredException(factoryName, ex);
            }
        }

        public Type GetFactoryType(string factoryName)
        {
            Type type;
            if (_cachedTypes.ContainsKey(factoryName))
            {
                type = _cachedTypes[factoryName];
            }
            else
            {
                type = Type.GetType(factoryName);
                if (type == null)
                {
                    throw new TypeLoadException($"Could not load a type with name: {factoryName}");
                }
                _cachedTypes.AddOrUpdate(factoryName, (a) => type, (a, b) => type);
            }
            return type;
        }

        public void ClearCache()
        {
            _cachedTypes.Clear();
        }

    }

Note it takes a Func<IServiceProvider> . This is created like so:

  FactoryDataPortal.FactoryLoader = new ServiceProviderObjectFactoryLoader(ServiceProviderCslaExtensions.GetServiceProviderForCurrentScope);

This means whenever ObjectFactory's are created, they will be resolved from the current scope.

I changed DbContextManager so that rather than create a new instance of DbContext which it then disposes of, it now resolves from current scope - and doesn't need to dispose.

 private DbContextManager(string database, string label)
        {
            _label = label;
            if (string.IsNullOrEmpty(database))
            {
                _context = ServiceProviderCslaExtensions.Locate<C>();

The nice thing about using LocalContext is that with your latest ContextManager implementation - this uses AsynLocal. So even when CSLA is used accross tasks that switch threads, the current scope flows with it.

Hopefully this might help someone who hits a similar scenario!

[rockfordlhotka]

This is interesting, and seems useful, thank you for sharing what you've found @dazinator.

It sounds like one easy way to improve CSLA in this regard would be to formalize a way for you to provide a Func (as part of configuration) that would be invoked to create a new instance of the DbContext when necessary. That would avoid your change to the core code.

[dazinator]

formalize a way for you to provide a Func (as part of configuration)

Yes I think that would be helpful, it would save having to implement a custom DbContextManager.
If you allowed a Func<> to be configured for creating the instance, I guess you would also need to allow a boolean to be specified which states whether the DbContextManager should dispose of the dbcontext or not on the last deref().

[rockfordlhotka]

I don't think it could be per-instance, it would have to be a static that's set on the type at the time of configuration. Otherwise that Func would be scattered all over your code.

[dazinator]

Agreed. So similar to object factory loader approach then

FactoryDataPortal.FactoryLoader = 

So something like:

DbContextManager.FactoryOptions = new DbContextFactoryOptions() {  FactoryMethod=myFunc, LeaveOpen=true}

Idea behind name LeaveOpen is stolen from Stream. MsDocs:

Unless you set the leaveOpen parameter to true, the StreamWriter object calls Dispose() on the provided Stream object when StreamWriter.Dispose is called.

So DbContextManager woukd honour LeaveOpen when it came to considering disposal of the dbcontext instance.

Going one step further, I do think there could be an opportunity to address this a bit more broadly..

Take this Func idea and maje it a Func<T> and that is configured at a global level like CslaServiceProvider.ResolveFunc = myFunc

Csla would then have a general purpose hook for container adaption / service resolution. For example in its default DbContextManagers, ObjectFactoryLoaders or anywhere else it needs to new up an instance of some type or resolve some services, it could use this to resolve the instance instead. The default / fallback implementation could do an Activator.CreateInstance() perhaps, to remain consistent with today.

This would then allow something like:

CslaServiceProvider.Func = myFunc
DbContextManager.FactoryOptions = new DbContextFactoryOptions() { LeaveOpen=true}

I have a feeling though that Csla could just use the System.IServiceProvider interface for this as it seems to be available very widely in .net..

ServiceProvider someProvider = BuildServiceProvider();
Csla.ApplicationContext.ServiceProvider = someProvider

If something like this was available in Csla then:

1. I would register my object factories with my ServiceProvider and would no longer need to implement a custom ObjectFactoryLoader as the default Csla one could now resolve using Csla's ServiceProvider.

2. I would register my DbContext with the ServiceProvider, and similar story for DbContextManager's.

With respect to scoping..

This complicates things. We would need the ability to override the IServiceProvider that Csla is using depending on current scope (Request, Hangfire job etc). You would end up coming back to having something similar to code I have shown above where you have the default ServiceProvider at a static (or in global context) level, and ability to set a scoped one that can be stored in LocalContext and which will override the global one when present.

Many containers today provide an IServiceProvider adaptor, but I am not certain its necessary for this to be used. Csla could define its own adaptor interface if it needed to.

Pretty wide change for Csla though so appreciate this kind if thing is not entered into lightly. A configurable func for dbcontextmanager would be better than nothing for now.

[rockfordlhotka]

I've been thinking about this quite a lot, including conversations with @JasonBock.

First, we need to understand that this will flow from Microsoft's IServiceProvider scheme - used in ASP.NET Core, but available to all .NET Core software. It is a common denominator.

Second, there are two scenarios to consider. We're only talking about server-side code here, invoked by the data portal, which means DP_XZY methods and ObjectFactory methods.

For ObjectFactory methods we can use constructor (ctor) injection, because those object instances are created, used, and destroyed specifically for the purpose of acting as a factory.

For DP_XYZ methods things are more complex, because we do not want to give references to these non-serializable/location-dependent objects (like database connections) to the business object as a whole - only to the DP_XYZ method(s) being invoked.

I think this means we need to write our own per-method DI implementation.

Typical IoC frameworks have ctor injection and property injection - but using either one of those will be bug-prone because I can basically guarantee that developers will forget to store those injected values in non-serializable properties/fields and the CSLA forum will be forever plagued by questions about why business object graphs refuse to serialize.

What we need is the ability for the data portal to use IoC concepts when invoking the DP_XYZ methods. Maybe these methods start allowing parameters that are injected?

  private void DataPortal_Fetch(int id, ICustomDal myDal) { }

In a sense the data portal already "injects" the criteria parameter. Conceptually we're talking about the data portal now being smart enough to look at the list of defined services from .NET Core and injecting anything that matches the other parameters people might add to their DP_XYZ methods.

I suspect it is harder than it sounds, but we need to agree on a high level design to explore before getting too far down the road.

Thoughts?

[dazinator]

For DP_XYZ methods things are more complex, because we do not want to give references to these non-serializable/location-dependent objects (like database connections) to the business object as a whole - only to the DP_XYZ method(s) being invoked.

Yes that's a key point..
The workaround to avoid this currently is to use the ServiceLocator anti-pattern within the DP_XYZ method - which is the method I have been using. This is typically where you want to get the current "scope" (i.e container) and locate any needed services (i.e DB Context) from that scope. This puts the responsibility of creating a mechanism for managing what the current scope / container is on the developer - most will create a singleton container instance and go from there to add checks for things like http request scope etc.
I am all for method injection in the dp_xyz methods then, because that should remove that concern from within the bodies of these methods such that they become simpler. However that concern would now have to be one for CSLA - i.e from which scope should it inject from? Providing some API in which the current scope (IServiceProvider) can be set would be good - this is similar to the approach I took above, i.e using LocalContext to store scopes that override the default in GlobalContext.

One minor thing - method injection doesn't solve the concern you mentioned entirely, because a developer could still grab an injected paramater and set a property ;-) - but it's still better, and perhaps that's a close as we can get!

[rockfordlhotka]

a developer could still grab an injected paramater and set a property

We can only hold people's hands to a certain point :)

[Chicagoan2016]

Does it mean we are going to abondon 'contrained construction' anti-pattern for plugable dataaccess in favor of more 'famous' DI containers?

Regards

[rockfordlhotka]

Personally I really like using a provider pattern when loading my DAL. But there's no doubt that DI simplifies the code in the DP_XYZ methods by shifting any loading of the DAL up into some other code, and the DAL is just provided as a parameter to your DP_XYZ method.

Following the ASP.NET Core DI model, basically you'll select your DAL in your app's startup.cs where you initialize the IoC container, and whatever DAL you've configured there is what'll be injected into the DP_XYZ methods.

I'm assuming encapsulated invocation - by far the most widely used data portal model. The other 3 models would inject database contexts or connections or something else.

Of course DI is a "turtles all the way down" thing - so the DAL object that's injected to DP_XYZ would also have been created by having the database connection/context injected into the DAL - again something defined on app startup in the IServiceCollection (typically ConfigureServices method).

[Chicagoan2016]

@rockfordlhotka ,
I couldn't get the Provider pattern working when we use namespace hierarchy between MyProject.DataAccess and MyProject.DataAccess.Sql.
The Interfaces defined in DataAccess project and Implementations in DataAccess.Sql have to at the top level namespace.

Regards

[JasonBock]

When @keithdv and I were thinking about Ystari, we spent a fair amount of time designing a way to do DI was operation-based, and also considered how to make this work with distributed scenarios. The notes are here, I'd suggest reading them as they're pretty relevant to this conversation.

https://github.com/MarimerLLC/Ystari/blob/master/docs/dependency_injection.md

[dazinator]

I'd think the best way to introduce this feature would be to allow an IServiceProvider to be set somewhere in csla application context. If this is present, then the server side DataPortal can use it to perform method injection of the DP_XYZ methods on the BO. If this is not present, then it can continue to behave as it does today.
Then depending on the host platform, if people want to leverage this feature, they'd have to make sure the CSLA IServiceProvider is set to the correct scoped IServiceProvider prior to BO DP_XYZ method invocation. For ASP.NET Core hosts, this means having a UseCsla() method that people can call in startup.cs that adds some CSLA middlware to the pipeline that takes the service provider from the HttpContext.RequestServices and sets it into the appropriate csla property, such that the server side dataportal will use it. For OWIN based hosts it would be a similar story. For .NET Core applications that aren't web based (i.e now that winforms and wpf are coming, or if its a console app etc) then the developer would still configure an IServiceProvider during app startup, but they could create new scoped providers as necessary (for example per winform etc) and set that scoped IServiceProvider into csla application context as appropriate. For WCF hosts on the .NET Framework, i've not looked at IServiceProvider integration.

I am already using this approach successfully, just without the method injection of the DP_XYZ calls - I am grabbing the current scoped IServiceProvider within my DP_XYZ method bodies and using it as a service locator.

One caveat of the method injection, it's not just registered services that would need to be injected, but also any object instances you passed directly into DataPortal.Fetch() - i.e such as Criteria. Microsoft.Extensions.DependencyInjection.ActivatorUtilities.CreateInstance() is an existing method in the framework that does what we would need, at the constructor injection level.

[rockfordlhotka]

@dazinator you say

I am grabbing the current scoped IServiceProvider within my DP_XYZ method bodies

How are you grabbing that object?

[rockfordlhotka]

This appears to be the heart of the code we'll need:

            var services = serviceCollection.BuildServiceProvider();

            var t = obj.GetType();
            var m = t.GetMethod("DoCoolStuff");
            var ps = m.GetParameters();

            var prms = new object [ps.GetLength(0)];
            for (int i = 0; i < ps.GetLength(0); i++)
            {
                var pType = ps[i].ParameterType;
                prms[i] = services.GetRequiredService(pType);
            }
            m.Invoke(obj, prms);

[dazinator]

@rockfordlhotka

How are you grabbing that object?

At the top of this thread, look at the ServiceProviderCslaExtensions class. I've given an explanation below it but to recap:

1. On startup, once I have the configured applications IServiceProvider I call:

serviceProvider.SetDefaultServiceProvider()

That extension method places it into csla's Global context.

Then, say I want to CSLA to use a scoped IServiceProvider rather than application one:

var someScopedServiceProvider = HttpContext.RequestServices();


someScopedServiceProvider.SetCurrentScopeServiceProvider():

That extension method puts the IServiceProvider into csla's LocalContext.

Then lastly there is a static method that I can use within csla DP_XYZ or anywhere else, to get the current service provider. It works by returning the one in LocalContext if present, falling back to GlobalContext.

In asp.net core applications I Use() middleware created for CSLA that at the start of a request does httpcontext.RequestServices.SetCurrentScopeServiceProvider();

[dazinator]

Oh other thing I found that works well using the above mechanism is unit testing. I can run xunit async tests in parallel rather than one at a time, thanks to the fact that LocalContext flows with async operations, each async unit test sets up an IServiceProvider with dependencies and then sets it into LocalContext using SetCurrentScopeServiceProvider().

[rockfordlhotka]

Based on the thread in #1102:

So the algorithm when criteria is provided needs to be something like:

1. Look for 1+ methods with the attribute where the first n parameters match the types of supplied criteria parameters
   1. If that list results in more than 1 method, throw an exception because the method is ambiguous
   2. If that list results in 0 methods, look for 1+ methods with the attribute where the first n parameters are object or match the types of supplied criteria parameters
      1. If that list results in more than 1 method, throw an exception because the method is ambiguous
      2. If that list results in 0 methods, throw an exception because no matching method could be found
2. Assuming no exception thus far, we have 1 method with matching criteria parameters
   1. Use the DI container to resolve instances for any/all remaining parameters
   2. Invoke the method, providing criteria + DI parameter values

The algorithm when NO criteria is provided needs to be something like:

1. Get a list of attributed methods
2. Eliminate the following:
   1. Methods with any parameter type that is not an interface
3. Find the method with the smallest number of parameters
   1. If more than one method remains, throw an exception because the result is ambiguous
4. Assuming no exception thus far, we have 1 method with 0+ interface-based parameters
   1. Use the DI container to resolve instances for any/all remaining parameters
   2. Invoke the method, providing DI parameter values

[dazinator]

I'd like to get @JasonBock's thoughts on this whole single vs multiple method discussion

Ok. I look forward to seeing what the outcome is for the next version of csla.

[rockfordlhotka]

@dazinator you and I are essentially reviewing the thought process that went into the implementation of the data portal we all know and love (?) today. Which is valuable, because that was all thought through back in 2001 or something 😄

I think what is making this change complex is that I'm looking at both DI and the multiple parameters from #1176 all at the same time.

[ajj7060]

@rockfordlhotka Sorry for any confusion; i'm 100% good with the attribute approach. I was just trying to reason out without it in case there was heavy opposition to it here.

I think without it though the scenario with Create() and Create(IMyCriteria) if the Create() never got resolved that'd be fine; just like today i might also have Create(string) and if I never call DP.Create(string) it'd never get called either.

I think though (if we're talking about no attribute, which I think isn't the way to go) if Csla tries to resolve IMyCriteria and the container actually returns null (some will throw if it can't resolve) Csla should throw and not pass null the the Create method.

So I'm left wondering. Perhaps we should say that there can be exactly and only one create/fetch/insert/update/delete/deleteself method per business class. Entirely do away with the concept of "empty" or no criteria or variations in criteria.

Well we actually do have multiple Fetchs/Creates now, to support slightly different ways to get the same data base. I'd really had to lose that, as places we've done that have been to simplify the Create/Fetch code as having one god fetch was overly complex.

So I am really liking the attribute to mark DI injected parameters. And if having multiple criteria parameters is marking things more complex, I think saying the DP methods can have zero or one criteria parameter as the first parameter in the method is fine... that's what we have today anyway.

Is there a value to allowing more than one criteria, when all this time we've been bundling things into a CriteriaBase<T> subclass all along anyway?

[ajj7060]

Does that mean for the super lazy you could provide a new general purpose FetchCriteria class out of the box that acted like a dictionary or even an expando / dynamic object?
I appreciate it's not the nicest and you loose compiled time checks and safeguards. I just suggest it because it almost gets you to being able to define a standard interface for the dp_ methods and everyone loves interfaces :-)

Sorry but I hate the idea of losing compile time static type checks 😄

[rockfordlhotka]

@ajj7060 you may want to look at that unit test code I linked to earlier in the thread - the current prototype implementation probably meets all your needs (I hope!). I took your suggestion and built the algorithm to be greedy, so these aren't ambiguous (even though they sort of are):

  [Create]
  private void Create() { }
  [Create]
  private void Create([FromService]IDal dal) { }

Both can technically handle the "no criteria" case, but because the algorithm is greedy it will consider this not ambiguous, and will select the second overload.

[ajj7060]

@rockfordlhotka ah ok, i will check that out tomorrow, thanks!

[rockfordlhotka]

Oh happiness! I now have a test that invokes a method with a combination of criteria and DI parameters!

This is coming together pretty nicely so far.

[dazinator]

Just chucking this out there.. another approach might be to serialise an expression tree and then `deseriliaze' it, compile and invoke it passing in the service provider as an argument.

End effect would be something like:

dp.Fetch((sp)=>this.GetById(1, "foo", sp.GetRequiredService<ILogger>());

I think that could do away with DataPortal_Xyz methods entirely and gain compile time checking as well as remove any ambiguity around which method is going to be called..

Probably a fair bit of work though. Looking at bottom answer here it should be possible: https://stackoverflow.com/questions/23253399/serialize-expression-tree

I'm pretty sure Hangfire (popular job runner) uses this approach for scheduling jobs.

[ajj7060]

@rockfordlhotka If I'm understanding the code, I think that covers everything that was discussed by everyone in the thread. Glad to hear its coming along nicely!

[rockfordlhotka]

Still need to finalize the attribute name.

[ajj7060]

Despite my earlier rant, I'm not too worried about the attribute name 😆 I think I like Inject, followed by Resolve, followed by FromService though. Hopefully if someone is reading the code Inject will immediately make them think of Dependency Injection. Resolve should too, but maybe not as immediately obvious, and FromService is good because that's what exists in Asp.net.

I'm really excited about these features, they will really make Csla easier to use I think!

[dazinator]

Great news @rockfordlhotka

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.