Is this interface too “god-like?”

Question

I have an MVC framework I've written. I'm trying to abstract out ASP.Net specific bits as well as make it more testable. Previously, I relied on HttpContext.Current in many places, which proves to be very unfriendly to unit testing. So, I started designing my own minimalistic interface for everything I'd need from there... However, it seems to keep on growing. This is how it currently looks:

/// <summary>
/// This is a very simplified context for server requests and responses
/// It should not cover "everything", but should instead have only what BarelyMVC needs to function
/// As a result, it will usually only have the most commonly used things in it
/// All "get" methods should normally return null if a given name/key doesn't exist
/// </summary>
public interface IServerContext
{
    /// <summary>
    /// Gets a cookie from the current request
    /// </summary>
    HttpCookie GetCookie(string name);
    /// <summary>
    /// Sets a cookie to send with the current response
    /// </summary>
    void SetCookie(HttpCookie cookie);
    /// <summary>
    /// Kills the current request. Kills it with fire! This function should never return(for security purposes)
    /// </summary>
    void KillIt();
    /// <summary>
    /// Gets the user agent sent by the current request
    /// </summary>
    string UserAgent{get;}
    /// <summary>
    /// Maps a relative URL path to an absolute path on the host (is this needed?)
    /// </summary>
    string MapPath(string path);
    /// <summary>
    /// Gets the client's IP address
    /// </summary>
    string UserIP{get;}
    /// <summary>
    /// Gets an HTTP header by name from the current request
    /// </summary>
    string GetHeader(string name);
    /// <summary>
    /// Sets an HTTP header to be sent back in the response to the current request
    /// </summary>
    void SetHeader(string name, string value);
    /// <summary>
    /// Performs a server-side transfer to another URL to service the current request (is this needed?)
    /// Should perform a KillIt() afterwards and never return
    /// </summary>
    void Transfer(string url);
    /// <summary>
    /// Send a 302 redirect back to the client for the given URL and use KillIt() to end the request
    /// </summary>
    void Redirect(string url);
    /// <summary>
    /// Gets a generic item from a key/value store associated with the current request only
    /// </summary>
    object GetItem(string name);
    /// <summary>
    /// Sets a generic item to a key/value store associated with the current request only
    /// </summary>
    object SetItem(string name, object value);
    /// <summary>
    /// Gets or sets the HTTP status code
    /// </summary>
    string HttpStatus{get;set;}
    /// <summary>
    /// Gets a TextWriter which can be used to send content back as the response
    /// </summary>
    TextWriter Writer{get;}
    /// <summary>
    /// Gets a parameter dictionary corresponding to the FORM values passed in by a POST request
    /// </summary>
    ParameterDictionary Form{get;}
    /// <summary>
    /// Gets the URL of the current request
    /// </summary>
    Uri RequestUrl{get;}
}

Is this interface already too big? I know I could abstract out the Request and Response parts, but I tend to think it make it a bit harder to use, especially because of my limits on "get" and "set" being stuck to request and response respectively.

What do you think? Refactoring into Request/Response interfaces or is it OK how it is?

Answer 1

You have to keep in mind that the earlier version of the .NET framework suffered in applying the interface segregation principle. In this case, the HTTP context includes methods associated with the request (ex. URL), the response (ex. HTTP status) and server utility functions (ex. map path).

With that in mind, the interface looks OK and familiar in terms of usage in comparison to the original HttpContext (System.Web) and HttpContextBase (System.Web.Mvc) and should be fine.

One aspect in regards to MVC that is hard to manage is the multiple data sources (query parameters, URL parameters, HTTP post Form values, routing parameters, session and the per request key/value store (HttpContext.Current.Items)) - perhaps it would be worthwhile to create an intermediary to manage these.

Answer 2

Right now your goal is to create a shim to make it easier to unit test code that interacts with HttpContext.Current. Since you add methods to this interface as you need them, it must meet your needs in that regard. So, keeping it as is definitely improves your code, and you should not change it unless you need to.

You may indeed need to change it. The interface segregation principle is really useful. If you are injecting this interface into other classes, you would get a lot more flexibility out of passing just the interface those classes need.

If you change this interface to use methods, and you only have one (or two, including a stub) implementations of this interface, refactoring toward a segregated model will be fairly easy with extension methods. Something like this:

public interface IServerContext
{
    IResponseDetails {get;}
    IRequestDetails {get;}
    //etc
}

public interface IRequestDetails
{
    Uri GetRequestUrl();
    //etc
}

public static IServerContextExtensions
{
    public static Uri GetRequestUrl(this IServerContext context)
    {
         return context.RequestDetails.GetRequestUrl();
    }
    //etc
}

Answer 3

As a framework author, you have the responsibility to do a lot of the heavy lifting for your users. Put yourself in their shoes. Ask what you'd like? Think about scenarios in which someone might need a request without a response or vis versa. Also note any properties/operations of HttpContext that are about the server (MapPath) and not related to requests or responses at all.