5
\$\begingroup\$

After asking a question on StackOverflow about the two .NET caching systems taking dependencies on each other I gave implementing them a go. I posted them as my own answer there but before accepting the answer I'd like some second opinions on the implementations.

HttpCacheChangeMonitor

Allows ObjectCache items to take a dependency on HTTPCache

public class HttpCacheChangeMonitor : ChangeMonitor
{
    private readonly string _uniqueId = Guid.NewGuid().ToString("N", CultureInfo.InvariantCulture);
    private readonly string[] _httpCacheKeys;

    public override string UniqueId
    {
        get { return _uniqueId; }
    }

    public HttpCacheChangeMonitor(string httpCacheKey)
        : this(new[] { httpCacheKey }) { }

    public HttpCacheChangeMonitor(string[] httpCacheKeys)
    {
        _httpCacheKeys = httpCacheKeys;
        Initialise();
    }

    private void Initialise()
    {
        HttpRuntime.Cache.Add(_uniqueId, _uniqueId, new CacheDependency(null, _httpCacheKeys), DateTime.MaxValue, Cache.NoSlidingExpiration, CacheItemPriority.NotRemovable, Callback);
        InitializationComplete();
    }

    private void Callback(string key, object value, CacheItemRemovedReason reason)
    {
        OnChanged(null);
    }

    protected override void Dispose(bool disposing)
    {
        Debug.WriteLine(
                _uniqueId + " notifying cache of change.", "HttpCacheChangeMonitor");
        HttpRuntime.Cache.Remove(_uniqueId);
    }
}

Test

public class HttpCacheChangeMonitorTests
{
    [Fact]
    public void ChangeMonitorTest()
    {
        HttpRuntime.Cache.Add("ChangeMonitorTest1", "", null, Cache.NoAbsoluteExpiration, new TimeSpan(0,10,0), CacheItemPriority.Normal, null);
        HttpRuntime.Cache.Add("ChangeMonitorTest2", "", null, Cache.NoAbsoluteExpiration, new TimeSpan(0, 10, 0), CacheItemPriority.Normal, null);
        using (MemoryCache cache = new MemoryCache("TestCache", new NameValueCollection()))
        {

            // Add data to cache
            for (int idx = 0; idx < 10; idx++)
            {
                cache.Add("Key" + idx, "Value" + idx, GetPolicy(idx));
            }

            long middleCount = cache.GetCount();

            // Flush cached items associated with "NamedData" change monitors
            HttpRuntime.Cache.Remove("ChangeMonitorTest1");

            long finalCount = cache.GetCount();

            Assert.Equal(10, middleCount);
            Assert.Equal(5, middleCount - finalCount);
            HttpRuntime.Cache.Remove("ChangeMonitorTest2");
        }
    }

    private static CacheItemPolicy GetPolicy(int idx)
    {
        string name = (idx % 2 == 0) ? "ChangeMonitorTest1" : "ChangeMonitorTest2";

        CacheItemPolicy cip = new CacheItemPolicy();
        cip.AbsoluteExpiration = System.DateTimeOffset.UtcNow.AddHours(1);
        cip.ChangeMonitors.Add(new HttpCacheChangeMonitor(name));
        return cip;
    }
}

MemoryCacheDependency

Allows HttpCache items to take a dependency on MemoryCache

public class MemoryCacheDependency : CacheDependency
{
    private readonly string _uniqueId = Guid.NewGuid().ToString("N", CultureInfo.InvariantCulture);
    private readonly IEnumerable<string> _cacheKeys;
    private readonly MemoryCache _cache;

    public override string GetUniqueID()
    {
        return _uniqueId;
    }

    public MemoryCacheDependency(MemoryCache cache, string cacheKey)
        : this(cache, new[] { cacheKey }) { }
    public MemoryCacheDependency(MemoryCache cache, IEnumerable<string> cacheKeys)
    {
        _cache = cache;
        _cacheKeys = cacheKeys;
        Initialise();
    }

    private void Initialise()
    {
        var monitor = _cache.CreateCacheEntryChangeMonitor(_cacheKeys);
        CacheItemPolicy pol = new CacheItemPolicy{AbsoluteExpiration = DateTime.MaxValue, Priority = CacheItemPriority.NotRemovable};
        pol.ChangeMonitors.Add(monitor);
        pol.RemovedCallback = Callback;
        _cache.Add(_uniqueId, _uniqueId, pol);
        FinishInit();
    }

    private void Callback(CacheEntryRemovedArguments arguments)
    {
        NotifyDependencyChanged(arguments.Source, EventArgs.Empty);
    }

    protected override void DependencyDispose()
    {
        Debug.WriteLine(
                   _uniqueId + " notifying cache of change.", "ObjectCacheDependency");
        _cache.Remove(_uniqueId);
        base.DependencyDispose();
    }
}

Test

public class MemoryCacheDependencyTests
{
    [Fact]
    public void CacheDependencyTest()
    {
        using (MemoryCache cache = new MemoryCache("TestCache", new NameValueCollection()))
        {
            cache.Add("HttpCacheTest1", DateTime.Now, new CacheItemPolicy {SlidingExpiration = new TimeSpan(0, 10, 0)});
            cache.Add("HttpCacheTest2", DateTime.Now, new CacheItemPolicy {SlidingExpiration = new TimeSpan(0, 10, 0)});

            // Add data to cache
            for (int idx = 0; idx < 10; idx++)
            {
                HttpRuntime.Cache.Add("Key" + idx, "Value" + idx, GetDependency(cache, idx), Cache.NoAbsoluteExpiration, new TimeSpan(0,10,0), CacheItemPriority.NotRemovable, null);
            }

            int middleCount = HttpRuntime.Cache.Count;

            // Flush cached items associated with "NamedData" change monitors
            cache.Remove("HttpCacheTest1");

            int finalCount = HttpRuntime.Cache.Count;

            Assert.Equal(10, middleCount);
            Assert.Equal(5, middleCount - finalCount);
        }
    }

    private static CacheDependency GetDependency(MemoryCache cache, int idx)
    {
        string name = (idx % 2 == 0) ? "HttpCacheTest1" : "HttpCacheTest2";

        return new MemoryCacheDependency(cache, name);
    }
}
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Overall looks very good, clean and well done. I only see minor tiny little things...

Nitpick - "Initialise" vs "Initialize"

This is just a minor nitpick, but it's kind of jumping at me:

private void Initialise()
{
    HttpRuntime.Cache.Add(_uniqueId, _uniqueId, new CacheDependency(null, _httpCacheKeys), DateTime.MaxValue, Cache.NoSlidingExpiration, CacheItemPriority.NotRemovable, Callback);
    InitializationComplete();
}

I find it weird that a method called Initialise() is calling one that's called InitializationComplete() - I think for consistency it would be better named Initialize().

Also the MSDN for InitializationComplete() says it's called from the constructor of derived classes to indicate that initialization is finished. - you are calling it from within the constructor, and I'm not familiar with using the ChangeMonitor class but it might be clearer to have that call explicitly show up in the constructor (similar to how InitializeComponents() remains in a form's constructor no matter what):

public HttpCacheChangeMonitor(string[] httpCacheKeys)
{
    _httpCacheKeys = httpCacheKeys;
    Initialize();
    InitializationComplete();
}

But then again it's no biggie, the code is very clear and easy to follow.


Temporal Coupling

That said I like constructors that do very little, and this is what you've got here, but the Initialise method seems of very little use, and has temporal coupling with the setting of _httpCacheKeys - I mean, if your constructor looked like this:

public HttpCacheChangeMonitor(string[] httpCacheKeys)
{
    Initialise();
    _httpCacheKeys = httpCacheKeys;
}

I'd expect it to blow up. One way to eliminate the temporal coupling, would be to take the array as a parameter:

private void Initialize(string[] httpCacheKeys)
{
    HttpRuntime.Cache.Add(_uniqueId, _uniqueId, new CacheDependency(null, httpCacheKeys), DateTime.MaxValue, Cache.NoSlidingExpiration, CacheItemPriority.NotRemovable, Callback);
    InitializationComplete();
}

...and then the order of operations doesn't matter anymore:

public HttpCacheChangeMonitor(string[] httpCacheKeys)
{
    Initialize(httpCacheKeys);
    _httpCacheKeys = httpCacheKeys;
}

(haven't looked at the test code)

\$\endgroup\$
1
  • \$\begingroup\$ Thanks Matt. On the first point using British English is automatic for me but I think you're right that consistency here is probably better. InitializationComplete is a toss up between explicitness and DRY. Initialize did have more use but I removed a couple of constructors so could probably revisit the need for it. \$\endgroup\$
    – Chao
    Apr 22, 2014 at 9:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.