ActionGroupContext to know an id of the current context

Question

My services(or services that depend on services) need to know about a current action group id. To add an extra optional parameter to each service for the id was an idea but in my current environment to difficult and ugly because every service would have to take in the parameter only to pass it to other services.

My solution was a class ActionGroupContext which stores the current context in a static ThreadStore or HttpStore.

What I don't like especially with my solution is that even though you are not do web application you have to include a reference to System.Web, but the convenience is great, the user does not need any setup code.

Do you have any improvement ideas?

Demo usage:

public void SomeApplicationMehtod()
{
    using (new ActionGroupContext())
    {
        someService.SomeMehtod();
    }
}

public class SomeService
{
    public void SomeMehtod()
    {
        var id = ActionGroupContext.Id;

        //Do something with the Id.

    }
}

The code:

public class ActionGroupContext : IDisposable
{
    private static readonly GuidStore GuidStore = new GuidStore("Action-Group-Id-Store");

    private readonly Guid? oldId;
    public ActionGroupContext()
    {
        oldId = GuidStore.GetGuid();
        GuidStore.SetGuid(Id ?? Guid.NewGuid());
    }

    public static Guid? Id
    {
        get { return GuidStore.GetGuid(); }
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (disposing)
        {
            GuidStore.SetGuid(oldId);
        }
    }
}

public class ThreadStore : IStore
{
    private readonly LocalDataStoreSlot slot;
    public ThreadStore(string storeKey)
    {
        slot = Thread.GetNamedDataSlot(storeKey);
    }

    public object GetData()
    {
        return Thread.GetData(slot);
    }

    public void SetData(object data)
    {
        Thread.SetData(slot, data);
    }
}

public interface IStore
{
    object GetData();
    void SetData(object data);
}

public class ThreadStore : IStore
{
    private readonly LocalDataStoreSlot slot;
    public ThreadStore(string storeKey)
    {
        slot = Thread.GetNamedDataSlot(storeKey);
    }

    public object GetData()
    {
        return Thread.GetData(slot);
    }

    public void SetData(object data)
    {
        Thread.SetData(slot, data);
    }
}

public class HttpStore : IStore
{
    private readonly string storeKey;

    public HttpStore(string storeKey)
    {
        if (!IsContextForStoreAvailable())
        {
            throw new InvalidOperationException("HttpContext is not available");
        }
        this.storeKey = storeKey;
    }

    public object GetData()
    {
        return HttpContext.Current.Items(storeKey);
    }

    public void SetData(object data)
    {
        HttpContext.Current.Items(storeKey) = data;
    }

    public static bool IsContextForStoreAvailable()
    {
        return HttpContext.Current != null;
    }
}

Answer 1

Your Dispose pattern is somehow flawed.

public void Dispose()
{
    Dispose(true);
    GC.SuppressFinalize(this);
}

protected virtual void Dispose(bool disposing)
{
    if (disposing)
    {
        GuidStore.SetGuid(oldId);
    }
}  

You don't check if the object already is disposed, so calling Dispose() over and over will result in each time calling GuidStore.SetGuid(oldId);.

So using a private variable for saving if the object is disposed would be the way to go.

private bool disposed = false;

public void Dispose()
{
    Dispose(true);
    GC.SuppressFinalize(this);
}

protected virtual void Dispose(bool disposing)
{
    if (!disposed && disposing)
    {
        GuidStore.SetGuid(oldId);
        disposed = true;
    }
}

Also for completeness you should also use the finalizer like so

~ActionGroupContext()
{
    Dispose(false);
}  

One thing what should never happen is that a call to Dispose() throws an exception. If you are 100% sure this can't happen, you are fine. If you aren't 100% sure, you should enclose the call to SetGuid() inside a try..catch block.

Please read this: proper-use-of-the-idisposable-interface

In your posted code I see neither in the ThreadStore nor in the HttpStore any usage of this ActionGroupContext so I see no reason to reference System.Web to use this context.