New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bootstrapper: Don't try to register types after instances have been requested #1227

Closed
dhilgarth opened this Issue Sep 13, 2013 · 43 comments

Comments

Projects
None yet
@dhilgarth

dhilgarth commented Sep 13, 2013

The current implementation mixes registrations with the container and resolves from the container.

Some DI containers are strict one-way: First registrations, afterwards resolving. There is no possibility to register a new type after at least one was resolved.

This is true for Simple Injector as well as - partially - for AutoFac. I see that in the Autofac bootstrapper a workaround has been used by creating a new container and using this to update the existing container, which is a hack IMHO.

Request: Change the initialization workflow in a way that first registers all types and only afterwards resolves them.

@thecodejunkie

This comment has been minimized.

Show comment
Hide comment
@thecodejunkie

thecodejunkie Sep 14, 2013

Member

I see that in the Autofac bootstrapper a workaround has been used by creating a new container and using this to update the existing container, which is a hack IMHO.

Are you talking about the request container? Where in the workflow do we register -> resolver -> register?

Member

thecodejunkie commented Sep 14, 2013

I see that in the Autofac bootstrapper a workaround has been used by creating a new container and using this to update the existing container, which is a hack IMHO.

Are you talking about the request container? Where in the workflow do we register -> resolver -> register?

@grumpydev

This comment has been minimized.

Show comment
Hide comment
@grumpydev

grumpydev Sep 14, 2013

Member

This simply isn't possible - we impose a very simple set of requirements on a container, which every container so far, (bar Windsor to an extent, because its child/sub container support is buggy) have been able to fit in with.

We do make a new builder and update the container with it in the Autofac bootstrapper, but as far as I know that's perfectly "allowed" and not a "hack" - we actually had Nicholas help us out with it at one point when we found a bug in Autofac itself.

If Simple Injector doesn't support what we need, and I've never heard of it so I've no idea if that's the case, then it can't be used, and I'd suggest this is an issue to raise with the Simple Injector folks :)

Member

grumpydev commented Sep 14, 2013

This simply isn't possible - we impose a very simple set of requirements on a container, which every container so far, (bar Windsor to an extent, because its child/sub container support is buggy) have been able to fit in with.

We do make a new builder and update the container with it in the Autofac bootstrapper, but as far as I know that's perfectly "allowed" and not a "hack" - we actually had Nicholas help us out with it at one point when we found a bug in Autofac itself.

If Simple Injector doesn't support what we need, and I've never heard of it so I've no idea if that's the case, then it can't be used, and I'd suggest this is an issue to raise with the Simple Injector folks :)

@grumpydev grumpydev closed this Sep 14, 2013

@dhilgarth

This comment has been minimized.

Show comment
Hide comment
@dhilgarth

dhilgarth Sep 14, 2013

If you say you can't - or don't want to - change the way it currently works, that's fine. I just want to make sure, that we are talking about the same thing here:
I am not talking about the request container, but about the following order of method calls on a bootstrapper:

GetApplicationContainer
RegisterBootstrapperTypes
RegisterTypes
RegisterCollectionTypes
RegisterInstances
GetApplicationRegistrationTasks
RegisterCollectionTypes // <<---

I wonder if the second call to RegisterCollectionTypes is expected.

Full call stacks can be found here

dhilgarth commented Sep 14, 2013

If you say you can't - or don't want to - change the way it currently works, that's fine. I just want to make sure, that we are talking about the same thing here:
I am not talking about the request container, but about the following order of method calls on a bootstrapper:

GetApplicationContainer
RegisterBootstrapperTypes
RegisterTypes
RegisterCollectionTypes
RegisterInstances
GetApplicationRegistrationTasks
RegisterCollectionTypes // <<---

I wonder if the second call to RegisterCollectionTypes is expected.

Full call stacks can be found here

@thecodejunkie

This comment has been minimized.

Show comment
Hide comment
@thecodejunkie

thecodejunkie Sep 14, 2013

Member

Yes, this is intentional because registrations can also be defined outside of the Bootstrapper, with the helper of the IApplicationRegistrations (or the helper implementation ApplicationRegistrations). In fact, RegisterTypes, RegisterCollectionTypes and RegisterInstances can be invoked 1-N times, depending on the numbers of IApplicationRegistrations/ApplicationRegistrations implementationts that Nancy find.

Like @grumpydev mentioned, this is working fine for us for all the containers we support out of the box (Autofac, Ninject, StructureMap, Unity and Windsor). With Windsor we have to do a slight trick, where we internally gather all of these up and perform a single registration when all calls are done. Depending on container requirements, you could perhaps do this as the first thing in the ApplicationStartup method.

The reason we do it like this is that the framework can provide it's defaults, while letting components / user-code override the implementations with their own registrations. In theory, we should be able to do something like this

var startups =
   this.GetApplicationStartupTasks().ToList();

var collectionTypeRegistrations = this.InternalConfiguration
   .GetCollectionTypeRegistrations()
   .Concat(this.GetApplicationCollections());

var allCollectionTypeRegistrations = 
   collectionTypeRegistrations.Concat(startups.SelectMany(x => x.CollectionTypeRegistrations));

this.RegisterCollectionTypes(this.ApplicationContainer, allCollectionTypeRegistrations);

But we would have to think about this for a bit to make sure we don't break anything. @grumpydev do you see any issues with optimizing it this way?

Member

thecodejunkie commented Sep 14, 2013

Yes, this is intentional because registrations can also be defined outside of the Bootstrapper, with the helper of the IApplicationRegistrations (or the helper implementation ApplicationRegistrations). In fact, RegisterTypes, RegisterCollectionTypes and RegisterInstances can be invoked 1-N times, depending on the numbers of IApplicationRegistrations/ApplicationRegistrations implementationts that Nancy find.

Like @grumpydev mentioned, this is working fine for us for all the containers we support out of the box (Autofac, Ninject, StructureMap, Unity and Windsor). With Windsor we have to do a slight trick, where we internally gather all of these up and perform a single registration when all calls are done. Depending on container requirements, you could perhaps do this as the first thing in the ApplicationStartup method.

The reason we do it like this is that the framework can provide it's defaults, while letting components / user-code override the implementations with their own registrations. In theory, we should be able to do something like this

var startups =
   this.GetApplicationStartupTasks().ToList();

var collectionTypeRegistrations = this.InternalConfiguration
   .GetCollectionTypeRegistrations()
   .Concat(this.GetApplicationCollections());

var allCollectionTypeRegistrations = 
   collectionTypeRegistrations.Concat(startups.SelectMany(x => x.CollectionTypeRegistrations));

this.RegisterCollectionTypes(this.ApplicationContainer, allCollectionTypeRegistrations);

But we would have to think about this for a bit to make sure we don't break anything. @grumpydev do you see any issues with optimizing it this way?

@dhilgarth

This comment has been minimized.

Show comment
Hide comment
@dhilgarth

dhilgarth Sep 14, 2013

The code in your answer would also perform a registration (RegisterCollectionTypes) after a resolve (GetApplicationStartupTasks), so it wouldn't really help.

I am thinking of a different solution: What about simply allowing to replace the container instance that Nancy stores in NancyBootstrapperBase<T>.ApplicationContainer?

dhilgarth commented Sep 14, 2013

The code in your answer would also perform a registration (RegisterCollectionTypes) after a resolve (GetApplicationStartupTasks), so it wouldn't really help.

I am thinking of a different solution: What about simply allowing to replace the container instance that Nancy stores in NancyBootstrapperBase<T>.ApplicationContainer?

@thecodejunkie

This comment has been minimized.

Show comment
Hide comment
@thecodejunkie

thecodejunkie Sep 14, 2013

Member

No it would perform a single call to RegisterCollectionTypes because it aggregates the GetCollectionTypeRegistrations(),GetApplicationCollections()and all the collection types from the startups, and performs a single call toRegisterCollectionTypes()`

Member

thecodejunkie commented Sep 14, 2013

No it would perform a single call to RegisterCollectionTypes because it aggregates the GetCollectionTypeRegistrations(),GetApplicationCollections()and all the collection types from the startups, and performs a single call toRegisterCollectionTypes()`

@dhilgarth

This comment has been minimized.

Show comment
Hide comment
@dhilgarth

dhilgarth Sep 14, 2013

Yeah, but GetApplicationStartupTasks performs a resolve:

        protected override IEnumerable<IApplicationStartup> GetApplicationStartupTasks()
        {
            return ApplicationContainer.GetAllInstances<IApplicationStartup>();
        }

and only afterwards is the registration inside RegisterCollectionTypes being performed.

dhilgarth commented Sep 14, 2013

Yeah, but GetApplicationStartupTasks performs a resolve:

        protected override IEnumerable<IApplicationStartup> GetApplicationStartupTasks()
        {
            return ApplicationContainer.GetAllInstances<IApplicationStartup>();
        }

and only afterwards is the registration inside RegisterCollectionTypes being performed.

@grumpydev

This comment has been minimized.

Show comment
Hide comment
@grumpydev

grumpydev Sep 14, 2013

Member

Yep, and it has to do a resolve because it can take dependencies, one of the reasons why this isn't possible to change. If you have an immutable container then you replace the instance when you do every registration or, preferably, have a batch operation that only recreates it once per batch. This is how the new .net immutable collections work, and its a pretty standard pattern. Immutability (which is what you sometimes need for perf/concurrency) is not the same as "you must do everything up front".

Member

grumpydev commented Sep 14, 2013

Yep, and it has to do a resolve because it can take dependencies, one of the reasons why this isn't possible to change. If you have an immutable container then you replace the instance when you do every registration or, preferably, have a batch operation that only recreates it once per batch. This is how the new .net immutable collections work, and its a pretty standard pattern. Immutability (which is what you sometimes need for perf/concurrency) is not the same as "you must do everything up front".

@dhilgarth

This comment has been minimized.

Show comment
Hide comment
@dhilgarth

dhilgarth Sep 14, 2013

Problem is: An immutable container doesn't seem to be supported by Nancy.
The Register* methods get the container passed in as an argument, so I can't replace it. Additionally, the ApplicationProperty is readonly.
Am I missing something here?

dhilgarth commented Sep 14, 2013

Problem is: An immutable container doesn't seem to be supported by Nancy.
The Register* methods get the container passed in as an argument, so I can't replace it. Additionally, the ApplicationProperty is readonly.
Am I missing something here?

@grumpydev

This comment has been minimized.

Show comment
Hide comment
@grumpydev

grumpydev Sep 14, 2013

Member

What i am saying is that if your container requires immutability then it should also provide a method to add things and replace the internal registrations.. Just like autofac does.

The only thing I can think of that we can do to.help with this is have the registration methods return the container, so we pass one in, and the method returns the updated one. For most implementations it will just mean returning the container that you have been passed, but the option is thee to construct a new one.

I still think this is a container design flaw though.

Member

grumpydev commented Sep 14, 2013

What i am saying is that if your container requires immutability then it should also provide a method to add things and replace the internal registrations.. Just like autofac does.

The only thing I can think of that we can do to.help with this is have the registration methods return the container, so we pass one in, and the method returns the updated one. For most implementations it will just mean returning the container that you have been passed, but the option is thee to construct a new one.

I still think this is a container design flaw though.

@dhilgarth

This comment has been minimized.

Show comment
Hide comment
@dhilgarth

dhilgarth Sep 14, 2013

Being able to return a new container would really be helpful, yes. And - as long as the container supports copying registrations from the existing one to a new one - this would solve all problems, I guess.

I certainly will talk to the developer of Simple Injector after I am clear on what you can and will do and what not, so we are not operating in a void here.

dhilgarth commented Sep 14, 2013

Being able to return a new container would really be helpful, yes. And - as long as the container supports copying registrations from the existing one to a new one - this would solve all problems, I guess.

I certainly will talk to the developer of Simple Injector after I am clear on what you can and will do and what not, so we are not operating in a void here.

@thecodejunkie

This comment has been minimized.

Show comment
Hide comment
@thecodejunkie

thecodejunkie Sep 14, 2013

Member

I just want to point out that all of this is just in one possible implementation of a Bootstrapper. Nancy really only knows about INancyBootstrapper and all the logic we are talking about here is introduced in NancyBootstrapperBase<T> for convenience when wrapping a container. It is perfectly possible to fallback and just implement the interface, should there be a need

Member

thecodejunkie commented Sep 14, 2013

I just want to point out that all of this is just in one possible implementation of a Bootstrapper. Nancy really only knows about INancyBootstrapper and all the logic we are talking about here is introduced in NancyBootstrapperBase<T> for convenience when wrapping a container. It is perfectly possible to fallback and just implement the interface, should there be a need

@dotnetjunkie

This comment has been minimized.

Show comment
Hide comment
@dotnetjunkie

dotnetjunkie Sep 15, 2013

As far as I can see, the only hack that has to be made for Simple Injector is to circumvent the IApplicationRegistrations registrations. This is probably not a really big deal, but if I may ask, what is your rational behind registering those IApplicationRegistrations implementations in the container?

Registering IApplicationRegistrations in the container seems odd to me, since those registrations have nothing to do with the application itself and the only one who depends on that interface is bootstrapper code; no application code will (or should) depend on that interface.

dotnetjunkie commented Sep 15, 2013

As far as I can see, the only hack that has to be made for Simple Injector is to circumvent the IApplicationRegistrations registrations. This is probably not a really big deal, but if I may ask, what is your rational behind registering those IApplicationRegistrations implementations in the container?

Registering IApplicationRegistrations in the container seems odd to me, since those registrations have nothing to do with the application itself and the only one who depends on that interface is bootstrapper code; no application code will (or should) depend on that interface.

@thecodejunkie

This comment has been minimized.

Show comment
Hide comment
@thecodejunkie

thecodejunkie Sep 15, 2013

Member

@dotnetjunkie They go into the container because they can have dependencies of their own. Granted that is more of a legacy thing because the IApplicationStartup and IApplicationRegistrations used to be the same interface. For startups it makes more sense to be able to take dependencies, than registrations .. though there might be cases where you'd want to make runtime decisions on which types to register from your IApplicationRegistrations //cc @grumpydev

Member

thecodejunkie commented Sep 15, 2013

@dotnetjunkie They go into the container because they can have dependencies of their own. Granted that is more of a legacy thing because the IApplicationStartup and IApplicationRegistrations used to be the same interface. For startups it makes more sense to be able to take dependencies, than registrations .. though there might be cases where you'd want to make runtime decisions on which types to register from your IApplicationRegistrations //cc @grumpydev

@dotnetjunkie

This comment has been minimized.

Show comment
Hide comment
@dotnetjunkie

dotnetjunkie Mar 27, 2015

Hi guys, would you mind adding an INancyModuleFactory (just like MVC's IControllerFactory) to the framework? This would massively simplify integrating Nancy with Simple Injector (or any DI container for that matter), because this allows Nancy to use its own DI container internally, while allowing users to intercept the way Nancy modules are built; which is usually the only place you actually need to intercept to integrate Nancy with any DI container, as Mark Seemann expresses in a general form here and here.

If adding a INancyModuleFactory is difficult or takes long to release, could you consider -as an intermediate solution- opening the DefaultNancyBootstrapper.GetModule(TinyIoCContainer, Type) method (i.e. removing the sealed keyword)? Just as with the INancyModuleFactory seem, overriding the GetModule method would simply allow users to redirect the creation of Nancy modules to a different container, again simplifying the integration drastically.

Please let me know what you think.

dotnetjunkie commented Mar 27, 2015

Hi guys, would you mind adding an INancyModuleFactory (just like MVC's IControllerFactory) to the framework? This would massively simplify integrating Nancy with Simple Injector (or any DI container for that matter), because this allows Nancy to use its own DI container internally, while allowing users to intercept the way Nancy modules are built; which is usually the only place you actually need to intercept to integrate Nancy with any DI container, as Mark Seemann expresses in a general form here and here.

If adding a INancyModuleFactory is difficult or takes long to release, could you consider -as an intermediate solution- opening the DefaultNancyBootstrapper.GetModule(TinyIoCContainer, Type) method (i.e. removing the sealed keyword)? Just as with the INancyModuleFactory seem, overriding the GetModule method would simply allow users to redirect the creation of Nancy modules to a different container, again simplifying the integration drastically.

Please let me know what you think.

@grumpydev

This comment has been minimized.

Show comment
Hide comment
@grumpydev

grumpydev Mar 27, 2015

Member

Nancy is already integrated with most of the popular DI frameworks, and it was pretty trivial in each case, so not sure about the "or any DI container for that matter" comment. Our bootstrapper base classes are just that, base classes, there's no requirement to actually use them to build off, the core interface is very straightforward, although you will have to consider your lifetime/scoping yourself.

I also don't agree that splitting the container up into an "internal" and an "external" one is a good idea - the whole idea behind the bootstrapper and container agnostic approach is to prevent that.

As for deferring construction of modules to an external container, that would require that the external container be aware of the current required scope, and would also have to have that scope disposed at the end of the request, so I'm not sure that would work (not to mention the confusion of configuring two containers and where things are resolved from - how would you resolve things like the context that must be registered per request for instance?).

At some point the bootstrapper base classes will change (or at least have a new one) to allow for making request level registrations "up front", rather than every request, to better serve containers that don't optimise registration times (not saying that's right or wrong), but there are still going to be some things that just have to be registered at the start of the request as they don't actually exist beforehand (and we're certainly not going down the thread local/call context/httpcontext nastyness :)). I can understand making a container immutable so you can optimise for resolution, but even if that were the case I'd still expect to be able to add things to it after it was setup, I'd just expect it to return a different "instance" each time (or just re-run whatever optimisations are necessary) - I think that's how AutoFac works, although I haven't poked around at that for a long time.

Member

grumpydev commented Mar 27, 2015

Nancy is already integrated with most of the popular DI frameworks, and it was pretty trivial in each case, so not sure about the "or any DI container for that matter" comment. Our bootstrapper base classes are just that, base classes, there's no requirement to actually use them to build off, the core interface is very straightforward, although you will have to consider your lifetime/scoping yourself.

I also don't agree that splitting the container up into an "internal" and an "external" one is a good idea - the whole idea behind the bootstrapper and container agnostic approach is to prevent that.

As for deferring construction of modules to an external container, that would require that the external container be aware of the current required scope, and would also have to have that scope disposed at the end of the request, so I'm not sure that would work (not to mention the confusion of configuring two containers and where things are resolved from - how would you resolve things like the context that must be registered per request for instance?).

At some point the bootstrapper base classes will change (or at least have a new one) to allow for making request level registrations "up front", rather than every request, to better serve containers that don't optimise registration times (not saying that's right or wrong), but there are still going to be some things that just have to be registered at the start of the request as they don't actually exist beforehand (and we're certainly not going down the thread local/call context/httpcontext nastyness :)). I can understand making a container immutable so you can optimise for resolution, but even if that were the case I'd still expect to be able to add things to it after it was setup, I'd just expect it to return a different "instance" each time (or just re-run whatever optimisations are necessary) - I think that's how AutoFac works, although I haven't poked around at that for a long time.

@khellang

This comment has been minimized.

Show comment
Hide comment
@khellang

khellang Mar 27, 2015

Member

It doesn't make sense to unseal the GetModule method of the DefaultNancyBootstrapper in the context of allowing other containers to resolve the modules. That's where the NancyBootstrapperWithRequestContainerBase<TContainer> comes in, which has an abstract GetModule method. The default bootstrapper is for the default container. If you want to use another container, you can derive from NancyBootstrapperWithRequestContainerBase<TContainer> 😄

Member

khellang commented Mar 27, 2015

It doesn't make sense to unseal the GetModule method of the DefaultNancyBootstrapper in the context of allowing other containers to resolve the modules. That's where the NancyBootstrapperWithRequestContainerBase<TContainer> comes in, which has an abstract GetModule method. The default bootstrapper is for the default container. If you want to use another container, you can derive from NancyBootstrapperWithRequestContainerBase<TContainer> 😄

@dotnetjunkie

This comment has been minimized.

Show comment
Hide comment
@dotnetjunkie

dotnetjunkie Apr 4, 2015

Hi Guys,

Thanks for your response. I’m glad you want to continue the discussion. What I’m hoping to get from this is either a consensus over some minimal adjustments/improvements to Nancy to make it easier to plug in any DI container into Nancy, or some knowledge about how to deal with this in the current version of Nancy.

Of the 6 most popular DI containers for .NET, Simple Injector is currently the only one missing from the list of adapters. I believe you when you say that implementing the other five containers was pretty trivial. After browsing through the code base though, I noticed there were quite some little tricks implemented into the adapters of the different DI containers. Unity for instance has a custom BuilderStrategy for handling enumerables; Windsor has a custom interceptor for handling request scoping; Ninject has special handling for dealing with Func<T> delegates. Besides that, I do remember you saying that you got help from Nicholas Blumhardt for supporting Autofac and you found that Windsor’s child/sub container support was buggy.

So even though most of that code was probably trivial to implement, it is still code that needs to be maintained; including all the little tricks that are in there. New releases of the supported DI containers will contain new features, and possible breaking changes. These might affect your adapter implementations and this might cause you to have packages for multiple major releases of a single container. And don’t forget that the popularity of containers change over time. e.g. The popularity of Simple Injector is increasing, while that of StructureMap is decreasing. And we don’t know how the world will look in 4 years. Perhaps LightInject and Dynamo are leading, while nobody uses Ninject anymore. This causes a burden on your team in maintaining all those adapters.

Although I’m not saying that you should throw away all the existing adapters (because that would break the code of all your users), this maintenance burden is something to take into consideration.

What I’m hoping is that you could make it easier for users to plug in the DI container –and version- of choice, or choose to refrain from using a DI container at all if they wish, and do Pure DI instead. There is a growing number of developers who live and breathe SOLID and DI but very consciously choose to wire things up by hand. This is a group of developers that should not be ignored IMO. We should not forget that a DI container is not a prerequisite for doing Dependency Injection.

What I like about Nancy is that what you yourself call the “Super-Duper-Happy-Path”. In the simplest form, it just works! You really can be productive in minutes! But this breaks down quickly when trying to use one of the more restrictive containers (or DI without container); this not only holds for Simple Injector. Letting Nancy do its stuff with its internal container and letting the user use a totally different container in the way he pleases, perfectly fits this strategy of this super-duper-happy-path. This prevents the details of how Nancy registers things from leaking out. As a user, I might not care about how Nancy manages its dependencies, and I might not really want to know.

My suggestion about having an IModuleFactory might have been a little simplistic, because request scoping must of course be taken into consideration. But two designs come to mind that solve this. If we look at MVCs IControllerFactory, we see that it has both a Create and Release method. This is the perfect place to start and end a scope, if scoping is needed. Another interesting solution is the use of a BeginScope() method as we see in Web API’s IDependencyResolver abstraction.

About the confusion of where to resolve things from; I don’t think this confusion actually exists. During the super-duper-happy-path, the user doesn’t need to inject any dependencies from Nancy, and each time they actually need such dependency, the user can resolve this dependency from Nancy’s container (or some Nancy supplied façade) and register it into the user’s container in a single line of code. In most applications however, you will probably not see more than a handful of these ‘cross-wired’ dependencies.

About the issue about things that must be registered per request: I must admit that throughout the years that I’m helping developers with dependency injection, I’ve never seen a case where injection of request specific services or runtime data couldn’t be prevented by changing (and I would even go as far as saying ‘improving’) the design of the user’s application. So even though Simple Injector and other restrictive containers don’t support per-request registrations (but do note that Simple Injector supports per-request lifestyles, which is something different), I have never seen this to be an issue for Simple Injector users. So laying this constraint on Nancy, simply because some users might want to do this, doesn’t seem fair to me. If users really need this (or IMO 'think' they do), they can always pick a container that supports this.

As I see it, having some IModuleFactory and IDependencyScope abstractions or something similar, would make it easier for a lot of developers, giving them much more freedom in how and if to use a container, and could give a consistent story about integrating with Nancy.

But as I said, overriding GetModule might work, although we need a strategy for dealing with scoping in that case. A simple overridable ExecuteModule(Type, NancyContext) perhaps? Do note that overriding the NancyBootstrapperWithRequestContainerBase<TContainer> doesn’t do the trick (as @khellang suggests), because this class lays some restrictions on the way the container works (as discussed 18 months back at the beginning of this thread).

I already experienced several Simple Injector users who switched back to their old HTTP framework after a failed attempt to integrate with Nancy, and I think it would be a shame if any other user of any of the more restrictive containers (or Pure DI practitioners) would refrain from using Nancy, because I think that Nancy is really neat.

I hope this all makes sense and I really like to hear from you what you think.

dotnetjunkie commented Apr 4, 2015

Hi Guys,

Thanks for your response. I’m glad you want to continue the discussion. What I’m hoping to get from this is either a consensus over some minimal adjustments/improvements to Nancy to make it easier to plug in any DI container into Nancy, or some knowledge about how to deal with this in the current version of Nancy.

Of the 6 most popular DI containers for .NET, Simple Injector is currently the only one missing from the list of adapters. I believe you when you say that implementing the other five containers was pretty trivial. After browsing through the code base though, I noticed there were quite some little tricks implemented into the adapters of the different DI containers. Unity for instance has a custom BuilderStrategy for handling enumerables; Windsor has a custom interceptor for handling request scoping; Ninject has special handling for dealing with Func<T> delegates. Besides that, I do remember you saying that you got help from Nicholas Blumhardt for supporting Autofac and you found that Windsor’s child/sub container support was buggy.

So even though most of that code was probably trivial to implement, it is still code that needs to be maintained; including all the little tricks that are in there. New releases of the supported DI containers will contain new features, and possible breaking changes. These might affect your adapter implementations and this might cause you to have packages for multiple major releases of a single container. And don’t forget that the popularity of containers change over time. e.g. The popularity of Simple Injector is increasing, while that of StructureMap is decreasing. And we don’t know how the world will look in 4 years. Perhaps LightInject and Dynamo are leading, while nobody uses Ninject anymore. This causes a burden on your team in maintaining all those adapters.

Although I’m not saying that you should throw away all the existing adapters (because that would break the code of all your users), this maintenance burden is something to take into consideration.

What I’m hoping is that you could make it easier for users to plug in the DI container –and version- of choice, or choose to refrain from using a DI container at all if they wish, and do Pure DI instead. There is a growing number of developers who live and breathe SOLID and DI but very consciously choose to wire things up by hand. This is a group of developers that should not be ignored IMO. We should not forget that a DI container is not a prerequisite for doing Dependency Injection.

What I like about Nancy is that what you yourself call the “Super-Duper-Happy-Path”. In the simplest form, it just works! You really can be productive in minutes! But this breaks down quickly when trying to use one of the more restrictive containers (or DI without container); this not only holds for Simple Injector. Letting Nancy do its stuff with its internal container and letting the user use a totally different container in the way he pleases, perfectly fits this strategy of this super-duper-happy-path. This prevents the details of how Nancy registers things from leaking out. As a user, I might not care about how Nancy manages its dependencies, and I might not really want to know.

My suggestion about having an IModuleFactory might have been a little simplistic, because request scoping must of course be taken into consideration. But two designs come to mind that solve this. If we look at MVCs IControllerFactory, we see that it has both a Create and Release method. This is the perfect place to start and end a scope, if scoping is needed. Another interesting solution is the use of a BeginScope() method as we see in Web API’s IDependencyResolver abstraction.

About the confusion of where to resolve things from; I don’t think this confusion actually exists. During the super-duper-happy-path, the user doesn’t need to inject any dependencies from Nancy, and each time they actually need such dependency, the user can resolve this dependency from Nancy’s container (or some Nancy supplied façade) and register it into the user’s container in a single line of code. In most applications however, you will probably not see more than a handful of these ‘cross-wired’ dependencies.

About the issue about things that must be registered per request: I must admit that throughout the years that I’m helping developers with dependency injection, I’ve never seen a case where injection of request specific services or runtime data couldn’t be prevented by changing (and I would even go as far as saying ‘improving’) the design of the user’s application. So even though Simple Injector and other restrictive containers don’t support per-request registrations (but do note that Simple Injector supports per-request lifestyles, which is something different), I have never seen this to be an issue for Simple Injector users. So laying this constraint on Nancy, simply because some users might want to do this, doesn’t seem fair to me. If users really need this (or IMO 'think' they do), they can always pick a container that supports this.

As I see it, having some IModuleFactory and IDependencyScope abstractions or something similar, would make it easier for a lot of developers, giving them much more freedom in how and if to use a container, and could give a consistent story about integrating with Nancy.

But as I said, overriding GetModule might work, although we need a strategy for dealing with scoping in that case. A simple overridable ExecuteModule(Type, NancyContext) perhaps? Do note that overriding the NancyBootstrapperWithRequestContainerBase<TContainer> doesn’t do the trick (as @khellang suggests), because this class lays some restrictions on the way the container works (as discussed 18 months back at the beginning of this thread).

I already experienced several Simple Injector users who switched back to their old HTTP framework after a failed attempt to integrate with Nancy, and I think it would be a shame if any other user of any of the more restrictive containers (or Pure DI practitioners) would refrain from using Nancy, because I think that Nancy is really neat.

I hope this all makes sense and I really like to hear from you what you think.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Dec 8, 2015

Why has this issue been closed ? It would be great to have simple injector which is one of the fastest DI containers. What @dotnetjunkie is suggesting makes perfect sense to me. Don't understand why you guys let this discussion die. Let's reach a consensus on this issue please.

ghost commented Dec 8, 2015

Why has this issue been closed ? It would be great to have simple injector which is one of the fastest DI containers. What @dotnetjunkie is suggesting makes perfect sense to me. Don't understand why you guys let this discussion die. Let's reach a consensus on this issue please.

@khellang

This comment has been minimized.

Show comment
Hide comment
@khellang

khellang Dec 8, 2015

Member

@ttgint The consensus is that with the way Nancy is built currently, there's no way to achieve this. Nancy relies on being able to "register, resolve and then register again" in order for its plugin architecture to work. AFAIK, Simple Injector is the only container (we've heard of) that have failed to comply with this requirement.

Nancy is also not the only web framework SI is incompatible with. Last time I checked, it wasn't usable with the DNX DI system either, so I imagine that SI won't be usable in the web sphere from now on. I would recommend you take a look at another container if you want to use it with Nancy or DNX 😄

We have, however, talked about rearchitecting the Nancy bootstrapper and its plugin infrastructure, but that won't happen in a while yet. We have bigger 🐟 to 🔥 right now.

Member

khellang commented Dec 8, 2015

@ttgint The consensus is that with the way Nancy is built currently, there's no way to achieve this. Nancy relies on being able to "register, resolve and then register again" in order for its plugin architecture to work. AFAIK, Simple Injector is the only container (we've heard of) that have failed to comply with this requirement.

Nancy is also not the only web framework SI is incompatible with. Last time I checked, it wasn't usable with the DNX DI system either, so I imagine that SI won't be usable in the web sphere from now on. I would recommend you take a look at another container if you want to use it with Nancy or DNX 😄

We have, however, talked about rearchitecting the Nancy bootstrapper and its plugin infrastructure, but that won't happen in a while yet. We have bigger 🐟 to 🔥 right now.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Dec 8, 2015

Fair enough @khellang cheers for the quick reply!

ghost commented Dec 8, 2015

Fair enough @khellang cheers for the quick reply!

@dotnetjunkie

This comment has been minimized.

Show comment
Hide comment
@dotnetjunkie

dotnetjunkie Dec 9, 2015

it wasn't usable with the DNX DI system either

As a matter of fact, we have an alpha release that is compatible with the new CoreCLR bits. (update: Since v3.1.2 Simple Injector officially supports CoreCLR)

so I imagine that SI won't be usable in the web sphere from now on

Wow, that's a stab in the back! Simple Injector is perfectly usable with ASP.NET 5, and as discussed here for instance, you don't need an adapter to do this. And if you read my previous response on this thread, this is precisely what I'm suggesting for Nancy. My suggestion is to add an IModuleFactory plus some sort of scoping abstraction. All problems would be solved when having those two abstractions (and they can even be merged into one). Solves for both Simple Injector as Pure DI users. Both abstractions are trivial for Simple Injector users to implement, and the module factory would become the composition root in case you're practicing Pure DI.

dotnetjunkie commented Dec 9, 2015

it wasn't usable with the DNX DI system either

As a matter of fact, we have an alpha release that is compatible with the new CoreCLR bits. (update: Since v3.1.2 Simple Injector officially supports CoreCLR)

so I imagine that SI won't be usable in the web sphere from now on

Wow, that's a stab in the back! Simple Injector is perfectly usable with ASP.NET 5, and as discussed here for instance, you don't need an adapter to do this. And if you read my previous response on this thread, this is precisely what I'm suggesting for Nancy. My suggestion is to add an IModuleFactory plus some sort of scoping abstraction. All problems would be solved when having those two abstractions (and they can even be merged into one). Solves for both Simple Injector as Pure DI users. Both abstractions are trivial for Simple Injector users to implement, and the module factory would become the composition root in case you're practicing Pure DI.

@khellang

This comment has been minimized.

Show comment
Hide comment
@khellang

khellang Dec 9, 2015

Member

As a matter of fact, we have an alpha release that is compatible with the new CoreCLR bits.

And this makes it compatible with MS.Ext.DI? CoreCLR != DNX 😄

Wow, that's a stab in the back!

Is it? If you fail to support any of the FOSS web frameworks out there, how is this not true? You think people will use something that's not supported? It's only my two cents.

Simple Injector is perfectly usable with ASP.NET 5, and as discussed here for instance, you don't need an adapter to do this.

Ah, so you have two containers composing your app? I must be mistaken, since other containers have adapters. I stand corrected.

My suggestion is to add an IModuleFactory plus some sort of scoping abstraction.

We would absolutely consider a PR. I've been down this road before, and it didn't end well. Nancy is simply not built for it. There are some assumptions made that are just not compatible with SI.

Member

khellang commented Dec 9, 2015

As a matter of fact, we have an alpha release that is compatible with the new CoreCLR bits.

And this makes it compatible with MS.Ext.DI? CoreCLR != DNX 😄

Wow, that's a stab in the back!

Is it? If you fail to support any of the FOSS web frameworks out there, how is this not true? You think people will use something that's not supported? It's only my two cents.

Simple Injector is perfectly usable with ASP.NET 5, and as discussed here for instance, you don't need an adapter to do this.

Ah, so you have two containers composing your app? I must be mistaken, since other containers have adapters. I stand corrected.

My suggestion is to add an IModuleFactory plus some sort of scoping abstraction.

We would absolutely consider a PR. I've been down this road before, and it didn't end well. Nancy is simply not built for it. There are some assumptions made that are just not compatible with SI.

@cemremengu

This comment has been minimized.

Show comment
Hide comment
@cemremengu

cemremengu Dec 9, 2015

Contributor

I guess we let @thecodejunkie have his PR and see what he can do on this issue given the recent changes he has been working on while the core guys focus on more important issues.

Contributor

cemremengu commented Dec 9, 2015

I guess we let @thecodejunkie have his PR and see what he can do on this issue given the recent changes he has been working on while the core guys focus on more important issues.

@thecodejunkie

This comment has been minimized.

Show comment
Hide comment
@thecodejunkie

thecodejunkie Dec 9, 2015

Member

Hi,

We'll gladly review any pull-request you guys send in. I will, however, raise your attention to the fact that any change at this level needs to be * firmly* verified with our existing Bootstrapper implementations before we would consider pulling anything in. I'll be honest here, this is not something we're currently going to prioritise doing ourselves, due to time constraints and having out focus on 2.x changes, so you'll have to take those steps yourself as part of this work.

Unfortunately these will be the circumstances here, but I'd rather be honest up front - we simply don't have the time to do it, ourselves, right now

Member

thecodejunkie commented Dec 9, 2015

Hi,

We'll gladly review any pull-request you guys send in. I will, however, raise your attention to the fact that any change at this level needs to be * firmly* verified with our existing Bootstrapper implementations before we would consider pulling anything in. I'll be honest here, this is not something we're currently going to prioritise doing ourselves, due to time constraints and having out focus on 2.x changes, so you'll have to take those steps yourself as part of this work.

Unfortunately these will be the circumstances here, but I'd rather be honest up front - we simply don't have the time to do it, ourselves, right now

@dotnetjunkie

This comment has been minimized.

Show comment
Hide comment
@dotnetjunkie

dotnetjunkie Dec 9, 2015

Great; let me see what I can come up with. I'm glad you're not discarding a design with IModuleFactory per definition.

dotnetjunkie commented Dec 9, 2015

Great; let me see what I can come up with. I'm glad you're not discarding a design with IModuleFactory per definition.

@khellang

This comment has been minimized.

Show comment
Hide comment
@khellang

khellang Dec 9, 2015

Member

I'm glad you're not discarding a design with IModuleFactory per definition.

The concept has just never been needed, since Nancy has always been an IoC-first framework. It relies on the container to do all composition. Nancy has never been a framework for "Pure DI"-people 😄

Member

khellang commented Dec 9, 2015

I'm glad you're not discarding a design with IModuleFactory per definition.

The concept has just never been needed, since Nancy has always been an IoC-first framework. It relies on the container to do all composition. Nancy has never been a framework for "Pure DI"-people 😄

@dotnetjunkie

This comment has been minimized.

Show comment
Hide comment
@dotnetjunkie

dotnetjunkie Dec 9, 2015

I'm not sure whether I missed something before, or whether some things have changed with Nancy since the last time I looked at it. However, as far as I can see now, the right abstractions are already in place to plugin Simple Injector, or use Pure DI without being forced in a registration API (a.k.a. Conforming Container). So I don't have to do any PR. I'll paste the code below and I would like the Nancy members to feedback on whether there are flaws in my approach.

The two abstractions that need to be replaced are INancyModuleCatalog and INancyContextFactory. The INancyContextFactory can be used to apply scoping, while the INancyModuleCatalog is in fact the INancyModuleFactory that I've been talking about before.

Here are the required implementations:

using System;
using System.Collections.Generic;
using System.Linq;
using Nancy;
using SimpleInjector;
using SimpleInjector.Lifestyles;

public sealed class SimpleInjectorModuleCatalog : INancyModuleCatalog
{
    private readonly Container container;
    public SimpleInjectorModuleCatalog(Container container) { this.container = container; }
    public INancyModule GetModule(Type moduleType, NancyContext context) =>
        (INancyModule)this.container.GetInstance(moduleType);
    public IEnumerable<INancyModule> GetAllModules(NancyContext context) =>
        from r in this.container.GetCurrentRegistrations()
        where typeof(INancyModule).IsAssignableFrom(r.ServiceType)
        select (INancyModule)r.GetInstance();
}

// Only required when you need to use Lifestyle.Scoped registrations
public sealed class SimpleInjectorScopedContextFactory : INancyContextFactory
{
    private readonly Container container;
    private readonly INancyContextFactory defaultFactory;

    public SimpleInjectorScopedContextFactory(Container container, INancyContextFactory @default) {
        this.container = container;
        this.defaultFactory = @default;
    }

    public NancyContext Create(Request request) {
        var context = this.defaultFactory.Create(request);
        context.Items.Add("SimpleInjector.Scope", AsyncScopedLifestyle.BeginScope(container));
        return context;
    }
}

As far as I see, this is all it takes. The SimpleInjectorModuleCatalog.GetInstance method simply delegates to Simple Injector, while the SimpleInjectorScopedContextFactory.Create begins a new scope (which enables per-request behavior) and registers the scope in the NancyContext.Items which allows the scope to end (be disposed) when the request ends.

With this code, we can now define the following Bootstrapper:

using Nancy;
using Nancy.Bootstrapper;
using Nancy.TinyIoc;
using SimpleInjector;
using SimpleInjector.Lifestyles;

public class Bootstrapper : DefaultNancyBootstrapper
{
    protected override void ApplicationStartup(TinyIoCContainer nancy, IPipelines pipelines) {
        // Create Simple Injector container
        Container container = new Container();
        container.Options.DefaultScopedLifestyle = new AsyncScopedLifestyle();

        // Register application components here, e.g.:
        container.Register(typeof(ICommandHandler<>), AppDomain.CurrentDomain.GetAssemblies());

        // Register Nancy modules.
        foreach (var nancyModule in this.Modules) container.Register(nancyModule.ModuleType);

        // Cross-wire Nancy abstractions that application components require (if any). e.g.:
        container.Register(nancy.Resolve<IModelValidator>);

        // Check the container.
        container.Verify();

        // Hook up Simple Injector in the Nancy pipeline.
        nancy.Register(typeof(INancyModuleCatalog), new SimpleInjectorModuleCatalog(container));
        nancy.Register(typeof(INancyContextFactory), new SimpleInjectorScopedContextFactory(
            container, nancy.Resolve<INancyContextFactory>()));
    }
}

The code seems to run fine, but I would like hear whether there are flaws in the code above.

dotnetjunkie commented Dec 9, 2015

I'm not sure whether I missed something before, or whether some things have changed with Nancy since the last time I looked at it. However, as far as I can see now, the right abstractions are already in place to plugin Simple Injector, or use Pure DI without being forced in a registration API (a.k.a. Conforming Container). So I don't have to do any PR. I'll paste the code below and I would like the Nancy members to feedback on whether there are flaws in my approach.

The two abstractions that need to be replaced are INancyModuleCatalog and INancyContextFactory. The INancyContextFactory can be used to apply scoping, while the INancyModuleCatalog is in fact the INancyModuleFactory that I've been talking about before.

Here are the required implementations:

using System;
using System.Collections.Generic;
using System.Linq;
using Nancy;
using SimpleInjector;
using SimpleInjector.Lifestyles;

public sealed class SimpleInjectorModuleCatalog : INancyModuleCatalog
{
    private readonly Container container;
    public SimpleInjectorModuleCatalog(Container container) { this.container = container; }
    public INancyModule GetModule(Type moduleType, NancyContext context) =>
        (INancyModule)this.container.GetInstance(moduleType);
    public IEnumerable<INancyModule> GetAllModules(NancyContext context) =>
        from r in this.container.GetCurrentRegistrations()
        where typeof(INancyModule).IsAssignableFrom(r.ServiceType)
        select (INancyModule)r.GetInstance();
}

// Only required when you need to use Lifestyle.Scoped registrations
public sealed class SimpleInjectorScopedContextFactory : INancyContextFactory
{
    private readonly Container container;
    private readonly INancyContextFactory defaultFactory;

    public SimpleInjectorScopedContextFactory(Container container, INancyContextFactory @default) {
        this.container = container;
        this.defaultFactory = @default;
    }

    public NancyContext Create(Request request) {
        var context = this.defaultFactory.Create(request);
        context.Items.Add("SimpleInjector.Scope", AsyncScopedLifestyle.BeginScope(container));
        return context;
    }
}

As far as I see, this is all it takes. The SimpleInjectorModuleCatalog.GetInstance method simply delegates to Simple Injector, while the SimpleInjectorScopedContextFactory.Create begins a new scope (which enables per-request behavior) and registers the scope in the NancyContext.Items which allows the scope to end (be disposed) when the request ends.

With this code, we can now define the following Bootstrapper:

using Nancy;
using Nancy.Bootstrapper;
using Nancy.TinyIoc;
using SimpleInjector;
using SimpleInjector.Lifestyles;

public class Bootstrapper : DefaultNancyBootstrapper
{
    protected override void ApplicationStartup(TinyIoCContainer nancy, IPipelines pipelines) {
        // Create Simple Injector container
        Container container = new Container();
        container.Options.DefaultScopedLifestyle = new AsyncScopedLifestyle();

        // Register application components here, e.g.:
        container.Register(typeof(ICommandHandler<>), AppDomain.CurrentDomain.GetAssemblies());

        // Register Nancy modules.
        foreach (var nancyModule in this.Modules) container.Register(nancyModule.ModuleType);

        // Cross-wire Nancy abstractions that application components require (if any). e.g.:
        container.Register(nancy.Resolve<IModelValidator>);

        // Check the container.
        container.Verify();

        // Hook up Simple Injector in the Nancy pipeline.
        nancy.Register(typeof(INancyModuleCatalog), new SimpleInjectorModuleCatalog(container));
        nancy.Register(typeof(INancyContextFactory), new SimpleInjectorScopedContextFactory(
            container, nancy.Resolve<INancyContextFactory>()));
    }
}

The code seems to run fine, but I would like hear whether there are flaws in the code above.

@jchannon

This comment has been minimized.

Show comment
Hide comment
@jchannon

jchannon Dec 11, 2015

Member

Strangely, I've decided to play around a PureDI approach today and I found the way to go forward is to implement my own INancyModuleCatalog. GetAllModules is I believe to tell the app about all the routes in all the modules. GetModule is called on the request and then the ModuleBuilder adds the Context,ModelValidator etc to the INancyModule.

I then register my implementation of INancyModuleCatalog in ConfigureApplicationContainer

 public class MyModuleCatalog : INancyModuleCatalog
    {
        private readonly Dictionary<Type, Func<INancyModule>> dicFactory; 

        public MyModuleCatalog()
        {
            dicFactory = new Dictionary<Type, Func<INancyModule>>
            {
                { typeof(EmptyModule), CreateEmptyModule },
                { typeof(HomeModule), CreateHomeModule }
            };
        }

        public System.Collections.Generic.IEnumerable<INancyModule> GetAllModules(NancyContext context)
        {
            foreach (var item in dicFactory.Values)
            {
                yield return item.Invoke();
            }
        }

        public INancyModule GetModule(System.Type moduleType, NancyContext context)
        {
            return this.dicFactory[moduleType].Invoke();
        }

        private INancyModule CreateEmptyModule()
        {
            return new EmptyModule();
        }

        private INancyModule CreateEmptyModule()
        {
            return new HomeModule();
        }

    }
Member

jchannon commented Dec 11, 2015

Strangely, I've decided to play around a PureDI approach today and I found the way to go forward is to implement my own INancyModuleCatalog. GetAllModules is I believe to tell the app about all the routes in all the modules. GetModule is called on the request and then the ModuleBuilder adds the Context,ModelValidator etc to the INancyModule.

I then register my implementation of INancyModuleCatalog in ConfigureApplicationContainer

 public class MyModuleCatalog : INancyModuleCatalog
    {
        private readonly Dictionary<Type, Func<INancyModule>> dicFactory; 

        public MyModuleCatalog()
        {
            dicFactory = new Dictionary<Type, Func<INancyModule>>
            {
                { typeof(EmptyModule), CreateEmptyModule },
                { typeof(HomeModule), CreateHomeModule }
            };
        }

        public System.Collections.Generic.IEnumerable<INancyModule> GetAllModules(NancyContext context)
        {
            foreach (var item in dicFactory.Values)
            {
                yield return item.Invoke();
            }
        }

        public INancyModule GetModule(System.Type moduleType, NancyContext context)
        {
            return this.dicFactory[moduleType].Invoke();
        }

        private INancyModule CreateEmptyModule()
        {
            return new EmptyModule();
        }

        private INancyModule CreateEmptyModule()
        {
            return new HomeModule();
        }

    }
@bbqchickenrobot

This comment has been minimized.

Show comment
Hide comment
@bbqchickenrobot

bbqchickenrobot Feb 2, 2016

As always @dotnetjunkie you rock!! Had a special case need to use Nancy (Linux) for it's routing purposes and as usual I would love to use SI (best DI in .net) and hopefully this will allow me. Otherwise I would have gone w/ writing my own routing system + SI.

bbqchickenrobot commented Feb 2, 2016

As always @dotnetjunkie you rock!! Had a special case need to use Nancy (Linux) for it's routing purposes and as usual I would love to use SI (best DI in .net) and hopefully this will allow me. Otherwise I would have gone w/ writing my own routing system + SI.

@BartoGabriel

This comment has been minimized.

Show comment
Hide comment
@BartoGabriel

BartoGabriel Oct 15, 2016

I am using OWIN + Nancy + SimpleInjector
I based on the solution of @dotnetjunkie and the SimpleInjector documentation (OwinIntegration)

Startup:

public class Startup
{
    public void Configuration(IAppBuilder app)
    {
        // Create Simple Injector container
        var container = new Container();
        container.Options.DefaultScopedLifestyle = new ExecutionContextScopeLifestyle();

        // Register application components here, e.g.:
        container.Register(typeof(ICommandHandler<>), AppDomain.CurrentDomain.GetAssemblies());

        // Begin and end the scope life
        app.Use(async (context, next) =>
        {
            using (container.BeginExecutionContextScope())
            {
                await next();
            }
        });

        using (container.BeginExecutionContextScope())
        {
            app.UseNancy(options => options.Bootstrapper = new Bootstrapper(container));
        }
    }
}

Bootstrapper:

public class Bootstrapper : DefaultNancyBootstrapper
{
    private readonly Container container;

    public Bootstrapper(Container container)
    {
        this.container = container;
    }

    protected override void ApplicationStartup(TinyIoCContainer nancy, IPipelines pipelines)
    {
        // Register Nancy modules.
        foreach (var nancyModule in Modules) container.Register(nancyModule.ModuleType);

        // Cross-wire Nancy abstractions that application components require (if any). e.g.:
        container.Register(nancy.Resolve<IModelValidator>);


        // Hook up Simple Injector in the Nancy pipeline.
        nancy.Register(typeof(INancyModuleCatalog), new SimpleInjectorModuleCatalog(container));
    }
}

SimpleInjectorModuleCatalog:

public sealed class SimpleInjectorModuleCatalog : INancyModuleCatalog
{
    private readonly Container _container;

    public SimpleInjectorModuleCatalog(Container container)
    {
        _container = container;
    }

    public INancyModule GetModule(Type moduleType, NancyContext context) =>
        (INancyModule) _container.GetInstance(moduleType);

    public IEnumerable<INancyModule> GetAllModules(NancyContext context) =>
        from r in _container.GetCurrentRegistrations()
        where typeof(INancyModule).IsAssignableFrom(r.ServiceType)
        select (INancyModule) r.GetInstance();
}

And I don't use SimpleInjectorScopedContextFactory, because the OWIN is the responsible of scope.

I have some doubts of my implementation:

  • I have to use BeginExecutionContextScope when you call the bootstrapper, because, nancy call to GetAllModules and thrown an exception because there is no scope.
using (container.BeginExecutionContextScope())
{
    app.UseNancy(options => options.Bootstrapper = new Bootstrapper(container));
}

This is correct? How could avoid call BeginExecutionContextScope?

  • Where can I call container.Verify()?
  • Is there a neater way to use OWIN and SimpleInjector?

BartoGabriel commented Oct 15, 2016

I am using OWIN + Nancy + SimpleInjector
I based on the solution of @dotnetjunkie and the SimpleInjector documentation (OwinIntegration)

Startup:

public class Startup
{
    public void Configuration(IAppBuilder app)
    {
        // Create Simple Injector container
        var container = new Container();
        container.Options.DefaultScopedLifestyle = new ExecutionContextScopeLifestyle();

        // Register application components here, e.g.:
        container.Register(typeof(ICommandHandler<>), AppDomain.CurrentDomain.GetAssemblies());

        // Begin and end the scope life
        app.Use(async (context, next) =>
        {
            using (container.BeginExecutionContextScope())
            {
                await next();
            }
        });

        using (container.BeginExecutionContextScope())
        {
            app.UseNancy(options => options.Bootstrapper = new Bootstrapper(container));
        }
    }
}

Bootstrapper:

public class Bootstrapper : DefaultNancyBootstrapper
{
    private readonly Container container;

    public Bootstrapper(Container container)
    {
        this.container = container;
    }

    protected override void ApplicationStartup(TinyIoCContainer nancy, IPipelines pipelines)
    {
        // Register Nancy modules.
        foreach (var nancyModule in Modules) container.Register(nancyModule.ModuleType);

        // Cross-wire Nancy abstractions that application components require (if any). e.g.:
        container.Register(nancy.Resolve<IModelValidator>);


        // Hook up Simple Injector in the Nancy pipeline.
        nancy.Register(typeof(INancyModuleCatalog), new SimpleInjectorModuleCatalog(container));
    }
}

SimpleInjectorModuleCatalog:

public sealed class SimpleInjectorModuleCatalog : INancyModuleCatalog
{
    private readonly Container _container;

    public SimpleInjectorModuleCatalog(Container container)
    {
        _container = container;
    }

    public INancyModule GetModule(Type moduleType, NancyContext context) =>
        (INancyModule) _container.GetInstance(moduleType);

    public IEnumerable<INancyModule> GetAllModules(NancyContext context) =>
        from r in _container.GetCurrentRegistrations()
        where typeof(INancyModule).IsAssignableFrom(r.ServiceType)
        select (INancyModule) r.GetInstance();
}

And I don't use SimpleInjectorScopedContextFactory, because the OWIN is the responsible of scope.

I have some doubts of my implementation:

  • I have to use BeginExecutionContextScope when you call the bootstrapper, because, nancy call to GetAllModules and thrown an exception because there is no scope.
using (container.BeginExecutionContextScope())
{
    app.UseNancy(options => options.Bootstrapper = new Bootstrapper(container));
}

This is correct? How could avoid call BeginExecutionContextScope?

  • Where can I call container.Verify()?
  • Is there a neater way to use OWIN and SimpleInjector?
@dotnetjunkie

This comment has been minimized.

Show comment
Hide comment
@dotnetjunkie

dotnetjunkie Oct 16, 2016

@BartoGabriel

How could avoid call BeginExecutionContextScope?

The only way to prevent this is by using a custom INancyContextFactory. When requesting all modules to build up the route cache, Nancy will call the INancyContextFactory get create a fake request. This will automatically start an execution context scope. Since Nancy builds up the cache before the first request starts, you will have to have an execution context scope. Simple Injector will prevent the creation of scoped objects outside an active scope.

Where can I call container.Verify()?

As last line in the Bootstrapper.ApplicationStartup method.

Is there a neater way to use OWIN and SimpleInjector?

AFAIK this is the neatest.

dotnetjunkie commented Oct 16, 2016

@BartoGabriel

How could avoid call BeginExecutionContextScope?

The only way to prevent this is by using a custom INancyContextFactory. When requesting all modules to build up the route cache, Nancy will call the INancyContextFactory get create a fake request. This will automatically start an execution context scope. Since Nancy builds up the cache before the first request starts, you will have to have an execution context scope. Simple Injector will prevent the creation of scoped objects outside an active scope.

Where can I call container.Verify()?

As last line in the Bootstrapper.ApplicationStartup method.

Is there a neater way to use OWIN and SimpleInjector?

AFAIK this is the neatest.

@bencyoung

This comment has been minimized.

Show comment
Hide comment
@bencyoung

bencyoung Jun 12, 2017

Just to note on this issue that the Update method used in the Autofac adaptor is deprecated. The message states that containers should not be updated once created...

bencyoung commented Jun 12, 2017

Just to note on this issue that the Update method used in the Autofac adaptor is deprecated. The message states that containers should not be updated once created...

@davidallyoung

This comment has been minimized.

Show comment
Hide comment
@davidallyoung

davidallyoung Jun 12, 2017

Contributor

@bencyoung This is a known issue and has had some discussion over here NancyFx/Nancy.Bootstrappers.Autofac#64 .

Contributor

davidallyoung commented Jun 12, 2017

@bencyoung This is a known issue and has had some discussion over here NancyFx/Nancy.Bootstrappers.Autofac#64 .

@bencyoung

This comment has been minimized.

Show comment
Hide comment
@bencyoung

bencyoung commented Jun 13, 2017

@davidallyoung Ah great, thanks!

@daveclarke

This comment has been minimized.

Show comment
Hide comment
@daveclarke

daveclarke Jun 14, 2017

WRT @dotnetjunkie post, BeginExecutionContext has been deprecated. The code changes based on the Simple Injector source is first in SingleInjectorModuleCatalog.cs the Create() method becomes:

    public NancyContext Create(Request request)
    {
        var context = this.defaultFactory.Create(request);
        context.Items.Add("SimpleInjector.Scope", SimpleInjector.Lifestyles.AsyncScopedLifestyle.BeginScope(container));
        return context;
    }

And in ApplicationStartup() the DefaultScopedLifestyle becomes:

        // Create Simple Injector container
        var container = new Container();
        container.Options.DefaultScopedLifestyle = new SimpleInjector.Lifestyles.AsyncScopedLifestyle();

Is that correct?

daveclarke commented Jun 14, 2017

WRT @dotnetjunkie post, BeginExecutionContext has been deprecated. The code changes based on the Simple Injector source is first in SingleInjectorModuleCatalog.cs the Create() method becomes:

    public NancyContext Create(Request request)
    {
        var context = this.defaultFactory.Create(request);
        context.Items.Add("SimpleInjector.Scope", SimpleInjector.Lifestyles.AsyncScopedLifestyle.BeginScope(container));
        return context;
    }

And in ApplicationStartup() the DefaultScopedLifestyle becomes:

        // Create Simple Injector container
        var container = new Container();
        container.Options.DefaultScopedLifestyle = new SimpleInjector.Lifestyles.AsyncScopedLifestyle();

Is that correct?

@dotnetjunkie

This comment has been minimized.

Show comment
Hide comment
@dotnetjunkie

dotnetjunkie Jun 15, 2017

@daveclarke yes, that is correct.

dotnetjunkie commented Jun 15, 2017

@daveclarke yes, that is correct.

@paulbir

This comment has been minimized.

Show comment
Hide comment
@paulbir

paulbir Aug 2, 2017

In Bootstrapper class this line
container.Register(typeof(ICommandHandler<>), AppDomain.CurrentDomain.GetAssemblies());
tell me that
Cannot resolve symbol ICommandHandler<>
Where is it defined?

paulbir commented Aug 2, 2017

In Bootstrapper class this line
container.Register(typeof(ICommandHandler<>), AppDomain.CurrentDomain.GetAssemblies());
tell me that
Cannot resolve symbol ICommandHandler<>
Where is it defined?

@paulbir

This comment has been minimized.

Show comment
Hide comment
@paulbir

paulbir Aug 2, 2017

@daveclarke Thanks. Comment above this line says: "Register application components". I thought it was something about Nancy components. It is just registering my types, right?

paulbir commented Aug 2, 2017

@daveclarke Thanks. Comment above this line says: "Register application components". I thought it was something about Nancy components. It is just registering my types, right?

@daveclarke

This comment has been minimized.

Show comment
Hide comment
@daveclarke

daveclarke Aug 2, 2017

@paulbir yes that's correct

daveclarke commented Aug 2, 2017

@paulbir yes that's correct

@paulbir

This comment has been minimized.

Show comment
Hide comment
@paulbir

paulbir Aug 3, 2017

Verifying fails with exception:
Additional information: The configuration is invalid. Creating the instance for type IModelValidator failed. The registered delegate for type IModelValidator threw an exception. Unable to resolve type: Nancy.Validation.IModelValidator
What's the problem?

paulbir commented Aug 3, 2017

Verifying fails with exception:
Additional information: The configuration is invalid. Creating the instance for type IModelValidator failed. The registered delegate for type IModelValidator threw an exception. Unable to resolve type: Nancy.Validation.IModelValidator
What's the problem?

@dotnetjunkie

This comment has been minimized.

Show comment
Hide comment
@dotnetjunkie

dotnetjunkie Aug 3, 2017

Hi @paulbir,

This is not the correct thread to ask this question. Please create a new issue with a Minimal, Complete, and Verifiable program and the full stacktrace either here at the Nancy forum or the Simple Injector forum.

dotnetjunkie commented Aug 3, 2017

Hi @paulbir,

This is not the correct thread to ask this question. Please create a new issue with a Minimal, Complete, and Verifiable program and the full stacktrace either here at the Nancy forum or the Simple Injector forum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment