webadvanced / Structuremap.MVC5

Apache License 2.0
21 stars 9 forks source link

Order of EndRequest events #5

Open MelGrubb opened 9 years ago

MelGrubb commented 9 years ago

I appreciate the fact that the StructureMap.MVC package now gives us a container-per-request pattern out of the box, but there is a problem with the implementation of StructureMapScopeModule. Because its Init method is hit so early in the lifecycle of the application, its subscription to the EndRequest method is the first one registered, trumping all others. In itself, this isn't a problem, but it means that the nested container gets disposed before the Application_EndRequest in Global.asax gets called. If you needed anything out of the container in that method, you'll just get a null reference exception because the container is long gone by then.

It is possible to work around this by using the main Container for stuff that happens in Global.asax, but in the case of things like logging something to a database, you'd now hold a database reference at the main container level, defeating the benefit of the nested container.

I could move the nested container disposal to Global.asax, and control the lifecycle from there, but that means individually and carefully merging any future updates to the Structuremap.MVC5 NuGet package, and re-applying this move.

Moving the nested container handling into a module does a nice job of making it "free" and invisible to developers, but causes problems in situations like this, which I don't think will be that uncommon. People want to clean things up in EndRequest. Often that will require items from the container. Hopefully you can find an approach that works here.

MelGrubb commented 9 years ago

Rather than just leaving the issue hanging like this, I'd like to proffer at least a suggestion. If the StructureMapScopeModule were implemented as a partial class, and added extension points for BeforeEndRequest and AfterEndRequest (RequestEnding, RequestEnded?), then users could implement their cleanup in that partial class and be protected from the potential package upgrade issues.

valmont commented 9 years ago

Hi Mel, this is a great point and issue I have not faced. The idea of a partial class is one I will play around with. I have always struggled with trying to give as much as I can for "free" without limiting developers that want to use the container for more advanced usage.

I am wondering if it would be too opinionated to have the package come with an IBeginRequest and IEndRequest interfaces. These would each have aint property named ExecutionOrder and void method named Execute. The StructureMapScopeModule would iterate through (in order) all items registered with those interfaces and call execute.

The app dev would need to use this convention for all code running in BeginRequest and EndRequest if they wanted to control the order.

Let me know your thoughts.

MelGrubb commented 9 years ago

I don't think an execution order parameter is a very good idea. If you have things that need to be done in a specific order, then you should define one top level "event" to coordinate that whole set of processes. Other events could still handle other unrelated things that don't depend on ordering.