Is it wrong to create an HttpContextProvider for use in IHttpModules?

Question

I have this legacy ASP.NET webapp that's mostly been transitioned to MVC. Code testability is generally bad, as it happens a lot to legacy apps: responsibilities are in a jumble, tight coupling prevails, that sort of thing.

I've been refactoring the app for a couple weeks to improve testability, doing my best to abide by SOLID, and I've come to this point where I need to pass HttpContext or HttpResponse and the like to a method in the HttpModule. Now, obviously the HttpModules can grab a copy of the HttpApplication and just pass that everywhere - which is how it is atm - but I would rather not grant access to such a large root object to a dependency.

Because some of our IHttpModules are initialized before the MVC even comes into play, I've had problems with using an IoC container to inject HttpContextBase or HttpResponseBase into constructors of various helpers that the modules use. I thus have to either pass the HttpContextBase to methods of said helpers - which is taxing to use and feels somewhat wrong - or create something like this:

public class HttpContextProvider : IHttpContextProvider {
    // ... ctor
    public HttpContextBase GetContext() { return HttpContext.Current; }
}

I would then inject an instance implementing IHttpContextProvider into a helper and call .GetContext() on it later as needed.

Intuitively, I feel like I might be doing something wrong, but cannot really get a handle on what it could be. This whole refactoring is complex enough - and the bus factor in the team is low enough - that I'm hesitant to request too much of the lead dev's time for this issue, which could almost be seen as academic from a practical standpoint.

Answer 1

I can suggest different options

1. try to register this objects in IoC, but to fix the problem (that class is initialized before it's dependences can be resolved) just use Lazy< T>, most IoC support them, check if yours is
2. Introducing providers as in your example, but I'll suggest it to return not HttpContext (or HttpResponse), but exactly what you need from this classes, it would be easier in tests to mock this behaviour (you maybe even don't need to construct context there at all) and also it will hide all complex stuff of HttpContext that isn't used

The second option will work only if you use this objects in similar way each time - so that you can replace it with 2-3 methods of ISmthProvider. If you use objects always differently maybe it will be better to combine both approaches and extract the most popular usages into some service and in other places path Lazy< HttpContextBase> to constructor

In my opinion it's better not to pass HttpContext/HttpResponse directly as it giving access for too much stuff and later people can make a mess of it (using it without thinking as it's already there) as a result it would be difficult to test and to make refactoring later.