C# cache controller

Problem

Currently my project utilises local caching wherever possible. There is a lot of similar looking code throughout that takes the form:

private static readonly ConcurrentDictionary<int, object> GetPMLocks = new ConcurrentDictionary<int, object>();
/// <summary>
/// Retrieve a PM by it's unique ID
/// </summary>
public static PrivateMessage GetPrivateMessage(int messageID)
{
    var cacheIndex = GetPMCacheIndex(messageID);
    var cache = GetCache();
    if (cache[cacheIndex == null])
    {
        try
        {
            GetPMLocks.TryAdd(messageID, new object());
            lock (CacheCommentLocks[messageID])
            {
                if (cache[cacheIndex] == null)
                {
                    using (var db = new DBContext())
                    {
                        var q = CompiledQueries.GetPMByID(db, messageID);
                        var pm = new PrivateMessage(q);

                        cache.Add(cacheIndex, pm, null,
                        Cache.NoAbsoluteExpiration,
                        Cache.NoSlidingExpiration,
                        CacheItemPriority.Normal,
                        null);
                    }
                }
            }
        }
        finally
        {
            object tR;
            GetPMLocks.TryRemove(messageID, out tR);
        }
    }
    return (PrivateMessage) cache[cacheIndex];
}

Solution

I wish to refactor this to be more concise, and to route every cache request through a central controller so it’s easy to fix bugs/modify & measure the caching process and performance. So I’ve created a new LocalCacheController class:

public class LocalCacheController
{
    private Cache CacheObj_ { get; set; }
    private Cache CacheObj {
        get
        {
            if (CacheObj_ == null) CacheObj_ = GetCache();
            return CacheObj_;
        }
    }

    /// <summary>
    /// Cache index this cacher is for
    /// </summary>
    private string ForCacheIndex { get; }

    public LocalCacheController(string cacheIndex, Cache cacheObj = null)
    {
        CacheObj_ = cacheObj;
        ForCacheIndex = cacheIndex;
    }

    /// <summary>
    /// Bulk save objects in cache if they don't exist.  Each tuple item needs an object to be saved, and a cache index
    /// </summary>
    public static void SetIfNotExists<T>(List<Tuple<T, string>> items)
    {
        // Nothing to do
        if (!items.Any()) return;

        var cache = GetCache();
        foreach (var item in items)
        {
            var controller = new LocalCacheController(item.Item2, cache);
            controller.SetIfNotExists(item.Item1);
        }
    }

    private static readonly ConcurrentDictionary<string, object> Locks = new ConcurrentDictionary<string, object>();

    /// <summary>
    /// Set a value if it doesn't exist
    /// </summary>
    /// <returns>Value in cache</returns>
    private T SetIfNotExists<T>(T obj)
    {
        SetLocalCacheValue(obj, false);
        return (T)CacheObj[ForCacheIndex];
    }

    /// <summary>
    /// Set value in cache.  If cache is empty, gets object from passed function
    /// </summary>
    /// <param name="getCachableObjectFunction">Function that returns the values to be cached</param>
    /// <returns>Value in cache either that already existed there or was just set</returns>
    public T SetIfNotExists<T>(Func<T> getCachableObjectFunction)
    {
        if (!IsCached(ForCacheIndex, CacheObj))
        {
            // We want to lock here, as we only wanted to ever call the 
            // passed function once as it can be expensive
            try
            {
                Locks.TryAdd(ForCacheIndex, new object());
                lock (Locks[ForCacheIndex])
                {
                    if (!IsCached(ForCacheIndex, CacheObj))
                    {
                        var obj = getCachableObjectFunction();
                        SetLocalCacheValue(obj, false);
                    }
                }
            }
            finally
            {
                object tR;
                Locks.TryRemove(ForCacheIndex, out tR);
            }
        }
        return (T)CacheObj[ForCacheIndex];
    }

    /// <summary>
    /// Sets a value in a cache regardless if it exists or not
    /// </summary>
    /// <returns>The object cached, or the object already in cache</returns>
    public T SetValue<T>(object objectToCache)
    {
        SetLocalCacheValue(objectToCache, true);
        return (T)CacheObj[ForCacheIndex];
    }

    /// <summary>
    /// Actually sets the value in the cache
    /// </summary>
    private void SetLocalCacheValue(object objectToCache, bool overwrite)
    {
        const CacheItemPriority priority = CacheItemPriority.Normal;
        var slidingExpiry = Cache.NoSlidingExpiration;
        var absoluteExpiry = Cache.NoAbsoluteExpiration;

        if (overwrite)
        {
            CacheObj.Insert(ForCacheIndex, objectToCache, null, absoluteExpiry, slidingExpiry, priority, null);
        }
        else
        {
            CacheObj.Add(ForCacheIndex, objectToCache, null, absoluteExpiry, slidingExpiry, priority, null);
        }
    }

    /// <summary>
    /// Is there an object in cache index
    /// </summary>
    public static bool IsCached(string cacheIndex, Cache cache = null)
    {
        if (cache == null) cache = GetCache();
        return cache[cacheIndex] != null;
    }

    /// <summary>
    /// Get cached bytes from local cache
    /// </summary>
    /// <param name="cacheIndex">Unique index to lookup</param>
    /// <param name="cache">Cache object if known</param>
    /// <returns>Null if cache index doesn't exist</returns>
    public static object GetFromLocalCache(string cacheIndex, Cache cache = null)
    {
        if (cache == null) cache = GetCache();
        if (cache[cacheIndex] == null) return null;
        return cache[cacheIndex];
    }

    /// <summary>
    /// Return cache object
    /// </summary>
    private static Cache GetCache()
    {
        return (HttpContext.Current == null)
                ? HttpRuntime.Cache
                : HttpContext.Current.Cache;
    }
}

Example Usages with Controller

1: Caching a single private message

This would replace the original “problem” code shown at the start of this post:

public static PrivateMessage GetPrivateMessage(int messageID)
{
    var cacheObj = new LocalCacheController(GetPMCacheIndex(messageID));
    var pm = cacheObj.SetIfNotExists(() => GetPM(messageID));
    return pm;
}
private static PrivateMessage GetPM(int messageID)
{
    using (var db = new DBContext())
    {
        var q = CompiledQueries.GetPMByID(db, messageID);
        if (q == null) return null;
        return new PrivateMessage(q);
    }
}

2: Given a list of private message ID’s, cache the uncached ones

public static void CacheUncachedMessageIDs(List<int> messageIDs)
{
    var uncached = messageIDs.Where(c => !LocalCacheController.IsCached(GetPMCacheIndex(c))).ToList();
    LocalCacheController.SetIfNotExists(GetPMs(uncached));
}
private static List<Tuple<PrivateMessage, string>> GetPMs(ICollection<int> messageIDs)
{
    var r = new List<Tuple<PrivateMessage, string>>();
    if (!messageIDs.Any()) return r;

    using (var db = new DBContext())
    {
        var q = db.PrivateMessages.Where(c => messageIDs.Contains(c.ID));
        foreach (var rec in q)
        {
            r.Add(new Tuple<PrivateMessage, string>(new PrivateMessage(rec), GetCacheIndex(rec.ID)));
        }
    }

    return r;
}

I believe I’ve achieved the objective of making the code more readable and concise. As this code is being relied on heavily throughout the project I would appreciate any input!

You have a lots of repetitions there:

private Cache CacheObj_ { get; set; }
private Cache CacheObj
{
  get
  {
      if (CacheObj_ == null) CacheObj_ = GetCache();
      return CacheObj_;
  }
}

and

private static Cache GetCache()
{
  return (HttpContext.Current == null)
          ? HttpRuntime.Cache
          : HttpContext.Current.Cache;
}

This can be all shortend to just:

private Cache Cache => HttpContext.Current?.Cache ?? HttpRuntime.Cache;

---

public LocalCacheController(string cacheIndex, Cache cacheObj = null)

You don’t need to pass the Cache object around. It’s actaully static so if you make the Cache propertey static too then you’re done.

private satatic Cache Cache => ...

---

public static void SetIfNotExists<T>(List<Tuple<T, string>> items)

As the use of the cache I don’t want to know if something already exists or not. I just want to add something to the cache and it should handle the exists/does not exist questions internally.

This should be just AddRange.

---

private string ForCacheIndex { get; }

This really needs a better name. It’s netiher an index nor a for. I suggest something like CacheKey as this looks like one.

---

public static object GetFromLocalCache(string cacheIndex, Cache cache = null)

As a user I don’t really want to know where the object comes from (unless there are more sources and here aren’t any other). A simple GetValue or GetObject would be enough. cacheIndex should be key.