Scoping resources per HTTP request

I am using this code in an MVC application to manage objects that should only have one instance per request.

/// <summary>
/// Manage all the resources that we want to scope to a single HTTP request.
/// </summary>
/// <remarks>
/// This class should not be directly referenced in many places.
/// In general, infrastructure code is OK whereas application code is not.
/// </remarks>
sealed class RequestScoped : IDisposable
{
    private static readonly object key = typeof(RequestScoped);

    /// <summary>
    /// Gets the RequestScoped instance for the given HttpContext.
    /// Creates and returns a new instance if it doesn't exist yet.
    /// This method is not thread-safe.
    /// The assumption is that there will only be one thread per request.
    /// </summary>
    public static RequestScoped GetOrCreate(System.Web.HttpContext context)
    {
        if (!context.Items.Contains(key))
        {
            context.Items[key] = new RequestScoped();
            context.AddOnRequestCompleted(Cleanup);
        }
        return (RequestScoped)context.Items[key];
    }

    private static void Cleanup(System.Web.HttpContext context)
    {
        if (context.Items.Contains(key))
        {
            var scope = (RequestScoped)context.Items[key];
            scope.Dispose();
        }
    }

    private readonly Lazy<IDbConnection> connection;
    private readonly Lazy<SessionRepository> sessionRepo;

    private RequestScoped()
    {
        connection = new Lazy<IDbConnection>(() => Utilities.MakeOpenConnection());
        sessionRepo = new Lazy<SessionRepository>(() => new SessionRepository(connection));
    }

    public IDbConnection OpenConnection { get { return connection.Value; } }
    public SessionRepository SessionRepo { get { return sessionRepo.Value; } }

    public void Dispose()
    {
        if (connection.IsValueCreated)
        {
            try
            {
                if (connection.Value.State == ConnectionState.Open)
                {
                    connection.Value.Close();
                }
            }
            catch (Exception)
            {
                // ignore
            }
            connection.Value.Dispose();
        }
    }
}

Example usage is like this:

protected IDbConnection OpenConnection
{
    get { return RequestScoped.GetOrCreate(HttpContext.Current).OpenConnection; }
}

(I am aware of DI containers but I have chosen not to use one in this application.)

Why is it specific to the two properties you have currently? I think it would make sense to make it more generic, but I can’t think of a good design to do that.

---

sealed class RequestScoped : IDisposable

Is it okay for users to call Dispose() by themselves? If not, don’t make this type IDisposable and don’t make Dispose() public.

---

private static readonly object key = typeof(RequestScoped);

Since other pieces of code have access to typeof(RequestScoped), they could potentially read or overwrite the RequestScoped instance that’s stored in the context by accident. To be on the safe side, I would use new object() here.

---

/// This method is not thread-safe.

I’m not sure you need to call this out here. Especially since this method is thread-safe, as long as different threads use different contexts.

---

public static RequestScoped GetOrCreate(System.Web.HttpContext context)

The fact that this method either gets existing object or creates a new one sounds like an implementation detail to me. I would probably call it just Get().

Why are you repeating the namespace for HttpContext instead of using using?

---

public IDbConnection OpenConnection { get { return connection.Value; } }

Is “open” here supposed to be a verb or an adjective? If verb, then that’s not appropriate for a property. If adjective, then it’s I think it’s not necessary.

---

public SessionRepository SessionRepo { get { return sessionRepo.Value; } }

I don’t see any reason to shorten “repository” to “repo” here, SessionRepository is not that long.

---

public void Dispose()
{
    if (connection.IsValueCreated)
    {
        try
        {
            if (connection.Value.State == ConnectionState.Open)
            {
                connection.Value.Close();
            }
        }
        catch (Exception)
        {
            // ignore
        }
        connection.Value.Dispose();
    }
}

Why are you not disposing SessionRepository? Is that not necessary?

Also, why are you trying to call Close() and then also call Dispose()? Isn’t just Dispose() enough?