ASP.NET MVC architecture questions

Question

I haven't done much .NET development in several years. Now I'm enjoying some of the new stuff with modern .NET. I just want to make sure I'm using things correctly. I'm a solo developer with no one else to bounce ideas off of.

I have a new MVC web application. I'm using the Massive micro-ORM. I'm a big believer in caching database requests. I've also created an EnumWrapper class.

Does anything look terrible in this code?

My caching class:

using System;
using System.Runtime.Caching;

namespace ULH.Common
{
    public static class Caching
    {
        private static readonly object _cacheLock = new object();

        public static dynamic LoadFromCache(string cacheKey, int secondsToCache, Func<object> query)
        {
            object result;

            if (secondsToCache == 0)
            {
                // if we're not caching, clear the cache and return result directly
                MemoryCache.Default.Remove(cacheKey);
                result = query.Invoke();
            }
            else
            {
                // otherwise do the caching, but use a double-check lock
                result = MemoryCache.Default[cacheKey];
                if (result == null)
                {
                    lock (_cacheLock)
                    {
                        result = MemoryCache.Default[cacheKey];
                        if (result == null)
                        {
                            result = query.Invoke();
                            MemoryCache.Default.Add(cacheKey, result, DateTime.UtcNow.AddSeconds(secondsToCache));
                        }
                    }
                }
            }

            return result;
        }

    }
}

An example of one of my data model objects:

using System;
using ULH.Common;

namespace ULH.Intranet.Chaplaincy
{
    public class EncounterTypes : BaseModel
    {
        private EncounterTypes()
        {
            this.TableName = "EncounterTypes";
            this.PrimaryKeyField = "Id";
        }

        public static dynamic GetActive()
        {
            return Caching.LoadFromCache("EncounterTypes_GetActive", 120,
                () => 
                {
                    dynamic tbl = new EncounterTypes();
                    return tbl.Find(Enabled: 1); 
                });
        }

        public static dynamic GetById(int id)
        {
            string cacheKey = "EncounterTypes_GetById_" + id.ToString();

            return Caching.LoadFromCache(cacheKey, 600,
                () =>
                {
                    dynamic tbl = new EncounterTypes();
                    return tbl.First(ID: id);
                });
        }

    }
}

Here is my EnumWrapper class, where I also cache the List that is created:

using System;
using System.Collections;
using System.Collections.Generic;

namespace ULH.Common
{

    public sealed class EnumWrapper : ReadOnlyCollectionBase
    {
        Type _enumType;
        Type _basetype;

        public static EnumWrapper Wrap(Type type)
        {
            if (type == null || !type.IsEnum)
                throw new ArgumentException("Type must be an enum", "type");

            return Caching.LoadFromCache("EnumWrapper:" + type.ToString() , 600,
                () =>
                {
                    return new EnumWrapper(type);
                });
        }

        private EnumWrapper(Type type)
        {
            _enumType = type;
            _basetype = Enum.GetUnderlyingType(type);

            foreach (var item in Enum.GetValues(_enumType))
            {
                this.InnerList.Add(
                    new KeyValuePair<object, string>(
                        Convert.ChangeType(item, _basetype), 
                        item.ToString()
                        )
                    );
            }
        }
    }
}

I'm using dynamic quite a bit due to the Massive ORM.

This won't be a huge system, so I don't want to deal with IOC containers. I'll have one common library (ULH.Common) with caching, the Massive ORM, EnumWrapper, and other common functions. There will then be several small websites using that library.

Suggestions? Does everything look okay?

Edit: George Howarth made some interesting points. I was unaware that MemoryCache was thread-safe. I've now modified my LoadFromCache() method to this:

public static dynamic LoadFromCache(string cacheKey, int secondsToCache, Func<object> query)
{
    object tocache = null;

    // result will always get the value here
    // tocache will only have the value when it was pulled from our method
    object result = MemoryCache.Default[cacheKey] ?? (tocache = query.Invoke());

    if (secondsToCache > 0)
    {
        if (tocache != null) // only save to cache if it wasn't there
            MemoryCache.Default.Add(cacheKey, tocache, DateTime.UtcNow.AddSeconds(secondsToCache));
    }
    else
    {
        // remove from cache only if secondsToCache was zero or less
        MemoryCache.Default.Remove(cacheKey);
    }

    return result;
}

Answer 1

• MemoryCache is actually thread-safe, so you don't need a double-check lock

  public static class Caching
  {
      public static dynamic LoadFromCache(string cacheKey, int secondsToCache, Func<object> query)
      {
          object result = query.Invoke();
  
          if (secondsToCache == 0)
          {
              // if we're not caching, clear the cache and return result directly
              MemoryCache.Default.Remove(cacheKey);
          }
          else
          {
              if (!MemoryCache.Default.Contains(cacheKey))
              {
                  MemoryCache.Default.Add(cacheKey, result, DateTime.UtcNow.AddSeconds(secondsToCache));
              }
          }
  
          return result;
      }
  }
  

Edit: Updated answer about the if statement, wasn't thinking straight.

Answer 2

I found a huge problem with the architecture I was working on. The cached object is still an object inheriting from DynamicModel - part of the Massive ORM.

This led to the database being queried even from a cached collection. That defeats the purpose of caching.

One way of handling this would be to add a new method to create ExpandoObjects (and collections of them) that did not inherit from DynamicModel. This seems more effort than it's worth. I'm still trying to determine my best course of action.