Better way to cache a Model

Question

My simple repository's getAll method:

    public List<ListModel> GetAllLists()
    {
            using (MySqlConnection connection = new MySqlConnection(this.connectionString))
            {
                return connection.Query<ListModel>("SELECT * FROM projectx.lists").AsList();
            }

    }

I'm using this class I've found here in so to handle caching:

    public class CacheUtils : ICacheService
    {    
        public TValue Get<TValue>(string cacheKey, Func<TValue> getItemCallback, double durationInMinutes = 120) where TValue : class
        {

            TValue item = MemoryCache.Default.Get(cacheKey) as TValue;
            if (item == null)
            {
                Debug.WriteLine("Not cached");
                item = getItemCallback();
                MemoryCache.Default.Add(cacheKey, item, DateTime.Now.AddMinutes(durationInMinutes));
            }
            else
                Debug.WriteLine("Cached!");
            return item;
        }

        public TValue Get<TValue, TId>(string cacheKeyFormat, TId id, Func<TId, TValue> getItemCallback, double durationInMinutes = 120) where TValue : class
        {

            string cacheKey = string.Format(cacheKeyFormat, id);
            TValue item = MemoryCache.Default.Get(cacheKey) as TValue;
            if (item == null)
            {

                item = getItemCallback(id);
                MemoryCache.Default.Add(cacheKey, item, DateTime.Now.AddMinutes(durationInMinutes));
            }


            return item;
        }
    }

Home controller:

    public ActionResult Index()
    {
        ListRepository listRep = new ListRepository();
        CacheUtils cache = new CacheUtils();
        return View(cache.Get("lists", listRep.GetAllLists));
    }

Question, is there a better way of handling cache than calling the helper from the controller? Ideally, it should be inside the repository method. But do I need to repeat the check for existing cache data on every single method of the repository? Ie.:

    public List<ListModel> GetAllLists()
    {
        var lists = Cache["lists"];
        if(lists == null)
        {
            using (MySqlConnection connection = new MySqlConnection(this.connectionString))
            {
                lists = connection.Query<ListModel>("SELECT * FROM projectx.lists").AsList();
            }

            Cache["lists"] = lists;
        }
        return ((List<ListModel>)lists);
    }

score 1 · Accepted Answer · edited May 23 '17 at 10:28

Use a decorator pattern and don't polute business or ui with caching logic. Wire it up with something like ninject (or poor bastards if you dont want to add a DI) I'd recommend marking it as single instance.

Benefits include:

Adding a invalidating method like void Save(ListModel) is easy to invalidate the cache.
Your top layer and bottom layer know nothing about the fact they have been cached.
You can also decorate again to add in logging, profiling, etc
You can also control the caching life cycle
you don't polute the controller level with caching logic
easy to remove

So something like the below would work. For how too add decorators in ninject see https://stackoverflow.com/a/8910599/1073280

public class MyHomeController
{
    private readonly IListCrud _listcrud;

    public MyHomeController(IListCrud listcrud)
    {
        _listcrud = listcrud;
    }

    public ActionResult Index()
    {
        return View(_listcrud.GetAllLists());
    }
}

public interface IListCrud
{
    List<ListModel> GetAllLists();
}

public class ListCrud : IListCrud
{
    public List<ListModel> GetAllLists()
    {
        using (MySqlConnection connection = new MySqlConnection(this.connectionString))
        {
            return connection.Query<ListModel>("SELECT * FROM projectx.lists").AsList();
        }
    }
}

public class ListCrudCache : IListCrud
{
    private readonly ICacheService _cache;
    private readonly IListCrud _inner;

    public ListCrudCache(ICacheService cache, IListCrud inner)
    {
        _cache = cache;
        _inner = inner;
    }

    public List<ListModel> GetAllLists()
    {
        return _cache.Get("lists", _inner.GetAllLists);
    }
}

Opinion: maybe just to keep the code small but be careful using select * with a ORM. if someone renames or removes a column you wont have any easy to unit test/detect failure mechanism.

score 0 · Answer 2 · answered Jul 26 '16 at 06:55

In my opinion it shouldn't be in the repository, as it (to me) smells like violation or SRP. Caching should be a higher level service above the repository.

You need to think about what actually needs the benefits of the caching. If the caching is for speeding up the WEB API interface, then having it in the controller is the best way by far. If you need caching elsewhere too, consider introducing some middle layer service classes and put caching there, but I would always make it optional in some way.

Better way to cache a Model

2 Answers2