ASP.NET MVC application architecture fit for unit testing with mocking

Question

I am trying to learn how to professionally work with unit testing, so I have refactored a medium sized project of mine (a sort of article aggregator that also does some NLP processing) and tried to apply some patterns to allow efficient testing (based on answers proposed here).

Current structure of the application:

1. Common assembly - contains generic functionalities, including models definition
2. Fetch job - daily job that fetches and processes raw article data
3. Web application - the actual Web application that allows users to see processed data
4. Test project - an automatic testing project based on NUnit that currently contains only integrative tests (no mocking)

My goal is to obtain an architecture to allow easy unit tests creation and also not to repeat my self in the process. Particularly, it is important because I want to apply what I learn on a significantly larger project that it is about to start.

Minimize EF connection string repetition - as I am using EF in multiple projects and it requires the connection string in its appropriate configuration file, I have tried the following approach:

• Define the connection string in a solution item:

  <?xml version="1.0" encoding="utf-8"?>
  
  <configuration>
    <appSetttings>
      <add key="ConnectionString" value='data source=localhost\sqlexpress2014;initial catalog=Aggregator;integrated security=True;multipleactiveresultsets=True;App=EntityFramework'/>
    </appSetttings>
  
  </configuration>
  

• Defined post build events for each project to copy the file in the output (depends on the project type, any way to be accessible at runtime):

• Connection string is read dynamically at runtime:

  public static String GetSolutionConfigurationItem(String configName)
  {
       String solutionItemPath = HttpContext.Current != null ? 
          HttpRuntime.AppDomainAppPath :
          Environment.CurrentDirectory;
       String solConfigPath = Path.Combine(solutionItemPath, SolutionConfigurationFile);
  
       var doc = XDocument.Load(solConfigPath);
       var configurationElem = doc.Element("configuration").Elements().ToList();
       var appSettingsElem = configurationElem[0];
       var configElem = appSettingsElem.Elements().FirstOrDefault(elem => elem.Attribute("key").Value.Equals(configName));
       var configValue = configElem.Attribute("value").Value;
       return configValue;
  }
  

Database context

public partial class AggregatorContext : IAggregatorContext
{
    #region Members
    private static Logger logger = LogManager.GetCurrentClassLogger();

    private static readonly object objSync = new object();
    private static readonly string DATACONTEXT_KEY = "AggregatorContext_UserInstance";
    private static readonly string DefaultConnectionString;

    private static string connectionString;
    #endregion

    static AggregatorContext()
    {
        String connString = Utils.GetSolutionConfigurationItem("ConnectionString");
        DefaultConnectionString = String.Format(
            @"metadata=res://*/AggregatorContext.csdl|res://*/AggregatorContext.ssdl|res://*/AggregatorContext.msl;provider=System.Data.SqlClient;provider connection string='{0}'", connString);
    }

    public AggregatorContext(String connectionString) : base(connectionString)
    {
    }

    /// <summary>
    /// Uses a UnitOfWorkStore to return the same instance of Context, that is unique
    /// per user, per postback (or CallContext, if not in an HttpContext environment, such as unit tests or console apps)
    /// </summary>
    public static AggregatorContext InstanceOfContext
    {
        get
        {
            // Dirty (non thread-safe) check
            if (UnitOfWorkStore.GetData(DATACONTEXT_KEY) == null)
            {
                lock (objSync)
                {
                    // Thread-safe check
                    if (UnitOfWorkStore.GetData(DATACONTEXT_KEY) == null)
                    {
                        AggregatorContext context = new AggregatorContext(DefaultConnectionString);
                        connectionString = context.Database.Connection.ConnectionString;
                        UnitOfWorkStore.SetData(DATACONTEXT_KEY, context);
                    }
                }
            }
            return (AggregatorContext)UnitOfWorkStore.GetData(DATACONTEXT_KEY);
        }
    }

    public static string ConnectionString
    {
        get { return connectionString; }
        set
        {
            // adding entity framework specific information, if not present
            StringBuilder sb = new StringBuilder();
            if (!value.EndsWith(";"))
                sb.Append(";");

            if (!value.Contains("multipleactiveresultsets"))
                sb.Append("multipleactiveresultsets=True;");

            if (!value.Contains("App=") || !value.Contains("Application"))
                sb.Append("App=EntityFramework");

            String actualConnectionString = value + sb.ToString();

            InstanceOfContext.Database.Connection.ConnectionString = actualConnectionString;
            connectionString = actualConnectionString;
        }
    }

    public bool SaveChangesEx()
    {
        try
        {
            SaveChanges();
            return true;
        }
        catch (DbEntityValidationException exc)
        {
            // just to ease debugging
            foreach (var error in exc.EntityValidationErrors)
            {
                foreach (var errorMsg in error.ValidationErrors)
                {
                    logger.LogEx(LogLevel.Error, "Error trying to save EF changes - " + errorMsg.ErrorMessage);
                }
            }

            return false;
            throw exc;
        }

    }
}

Using unit of work pattern to ensure I have a single instance of Context per request:

public class UnitOfWork : IUnitOfWork
{
    #region Members
    private IAggregatorContext _context;
    #endregion

    #region Properties
    public Lazy<IRepository<Lexem>> LexemRepository { get; private set; }
    public Lazy<IRepository<Word>> WordRepository { get; private set; }
    public Lazy<IRepository<Synset>> SynsetRepository { get; private set; }
    #endregion

    #region Constructor
    public UnitOfWork(IAggregatorContext context)
    {
        this._context = context;

        LexemRepository = new Lazy<IRepository<Lexem>>(() => new Repository<Lexem>(context));
        WordRepository = new Lazy<IRepository<Word>>(() => new Repository<Word>(context));
        SynsetRepository = new Lazy<IRepository<Synset>>(() => new Repository<Synset>(context));
    }
    #endregion

    #region Methods
    public IRepository<T> GetRepository<T>()
        where T: class
    {
        Type thisType = this.GetType();
        foreach (var prop in thisType.GetProperties())
        {
            var propType = prop.PropertyType;
            if (propType.GetGenericTypeDefinition() != typeof(Lazy<>))
                continue;

            var repoType = propType.GenericTypeArguments[0];
            if (!typeof(IRepository).IsAssignableFrom(repoType))
                continue;

            var objectType = repoType.GenericTypeArguments[0];
            if (objectType == typeof(T))
                return ((Lazy<IRepository<T>>) prop.GetValue(this)).Value;
        }

        throw new ArgumentException(String.Format("No repository of type {0} found", typeof(T).FullName));
    }

    public void SaveChanges()
    {
        _context.SaveChanges();
    }

    public bool SaveChangesEx()
    {
        return _context.SaveChangesEx();
    }
    #endregion
}

The repository class definition:

public class Repository<T> : IRepository<T> where T : class, new()
{
    private IAggregatorContext _context;

    public Repository(IAggregatorContext context)
    {
        this._context = context;
    }

    public IQueryable<T> All
    {
        get { return _context.Set<T>().AsQueryable(); }
    }

    public IQueryable<T> AllNoTracking
    {
        get { return _context.Set<T>().AsNoTracking(); }
    }

    public T Get(int id)
    {
        return _context.Set<T>().Find(id);
    }

    public void Delete(T entity)
    {
        if (_context.Entry(entity).State == EntityState.Detached)
            _context.Set<T>().Attach(entity);
        _context.Set<T>().Remove(entity);
    }

    public void Insert(T entity)
    {
        _context.Set<T>().Add(entity);
    }

    public void Update(T entity)
    {
        _context.Set<T>().Attach(entity);
        _context.Entry(entity).State = EntityState.Modified;
    }
}

Dependency injection - is used via Ninject to also allow redefining in test project:

• Context provider for the database context:

  public class AggregatorContextProvider : IInstanceProvider
  {
      public object GetInstance(Ninject.Extensions.Factory.Factory.IInstanceResolver instanceResolver, System.Reflection.MethodInfo methodInfo, object[] arguments)
      {
          var instance = AggregatorContext.InstanceOfContext;
          return methodInfo.Invoke(instance, arguments);
      }
  }
  

• Bindings definition in the job:

     public class FetchJobModule : NinjectModule
     {
         public override void Load()
         {
             Bind<IUnitOfWork>().To<UnitOfWork>();
             Bind<IAggregatorContext>().ToFactory(() => new AggregatorContextProvider());
             Bind<IFetchJob>().To<FetchJob>().InSingletonScope();
  
             Bind<IArticleParser>().ToProvider<ArticleParserProvider>();
         }
      }
  
      public class ArticleParserProvider : IProvider
      {
     public object Create(IContext context)
     {
      // looking for a ISource parameter in context chain
      var currContext = context as Ninject.Activation.Context;
      IArticleSource source = null;
      while (currContext != null)
      {
          var sourceParam = currContext.Parameters.SingleOrDefault(p => p.Name.Equals("source"));
  
          if (sourceParam != null)
          {
              source = (IArticleSource)sourceParam.GetValue(currContext, null);
              break;
          }
  
          if (currContext.Request == null)
              break;
  
          currContext = currContext.Request.ParentContext as Ninject.Activation.Context;
      }
  
      if (source == null)
          throw new Ninject.ActivationException("Could not create article parser - source is missing from context chain");
  
      var unitOfWork = context.Kernel.Get<IUnitOfWork>();
      var nlpUtils = context.Kernel.Get<INlpUtils>(new Ninject.Parameters.ConstructorArgument("source", source));
  
      if (source.Name.Equals(Constants.ArticleSources.S1))
          return new S1ArticleParser(unitOfWork, source, nlpUtils);
      if (source.Name.Equals(Constants.ArticleSources.S2))
          return new S2ArticleParser(unitOfWork, source, nlpUtils);
  
      throw new NotSupportedException(String.Format("Unsupported source: {0}", source.Name));
  }
  
  public Type Type
  {
      get { return typeof(ArticleParser); }
  }
  }
  

ArticleParserProvider is really ugly as my parsers inherit from ArticleParser and DI favours composition over inheritance.

The job is a simple console application, so DI is set up at startup:

class Program
{
    #region Members
    private static Logger logger = LogManager.GetCurrentClassLogger();
    #endregion

    #region Properties
    private static IUnitOfWork _UnitOfWork { get; set; }
    private static IKernel _Kernel { get; set; }
    private static IFetchJob _FetchJob { get; set; }
    #endregion

    #region Methods
    private static void init()
    {
        // setup DI
        _Kernel = new StandardKernel();
        _Kernel.Load(Assembly.GetExecutingAssembly());

        _UnitOfWork = _Kernel.Get<IUnitOfWork>();
        _FetchJob = _Kernel.Get<IFetchJob>();
    }

    static void Main(string[] args)
    {
        init();

        Utils.InitNLogConnection();
        logger.LogEx(LogLevel.Info, "FetchJob started");

        // ...
    }
    #endregion
}

• Bindings definition in the Web application:

  [assembly: WebActivatorEx.PreApplicationStartMethod(typeof(NewsAggregator.Web.App_Start.NinjectWebCommon), "Start")]
  [assembly: WebActivatorEx.ApplicationShutdownMethodAttribute(typeof(NewsAggregator.Web.App_Start.NinjectWebCommon), "Stop")]
  
  namespace NewsAggregator.Web.App_Start
  {
  // usings stripped for brevity
  
  public static class NinjectWebCommon 
  {
      private static readonly Bootstrapper bootstrapper = new Bootstrapper();
  
      /// <summary>
      /// Starts the application
      /// </summary>
      public static void Start() 
      {
          DynamicModuleUtility.RegisterModule(typeof(OnePerRequestHttpModule));
          DynamicModuleUtility.RegisterModule(typeof(NinjectHttpModule));
          bootstrapper.Initialize(CreateKernel);
      }
  
      /// <summary>
      /// Stops the application.
      /// </summary>
      public static void Stop()
      {
          bootstrapper.ShutDown();
      }
  
      /// <summary>
      /// Creates the kernel that will manage your application.
      /// </summary>
      /// <returns>The created kernel.</returns>
      private static IKernel CreateKernel()
      {
          var kernel = new StandardKernel();
          try
          {
              kernel.Bind<Func<IKernel>>().ToMethod(ctx => () => new Bootstrapper().Kernel);
              kernel.Bind<IHttpModule>().To<HttpApplicationInitializationHttpModule>();
  
              RegisterServices(kernel);
              return kernel;
          }
          catch
          {
              kernel.Dispose();
              throw;
          }
      }
  
      /// <summary>
      /// Load your modules or register your services here!
      /// </summary>
      /// <param name="kernel">The kernel.</param>
      private static void RegisterServices(IKernel kernel)
      {
          kernel.Bind<IAggregatorContext>().ToFactory(() => new AggregatorContextProvider());
          kernel.Bind<IUnitOfWork>().To<UnitOfWork>().InRequestScope();
  
      }        
  }
  }
  

• Bindings definition in the Test project are similar to those in the Web application.

How can this be improved to fit my goal (unit testing with mocking and as few repetitions as possible)?

Answer 1

Dependency Injection doesn't work if you half ass it only take it half way. Consider this ctor.

    #region Constructor
    public UnitOfWork(IAggregatorContext context)
    {
        this._context = context;

        LexemRepository = new Lazy<IRepository<Lexem>>(() => new Repository<Lexem>(context));
        WordRepository = new Lazy<IRepository<Word>>(() => new Repository<Word>(context));
        SynsetRepository = new Lazy<IRepository<Synset>>(() => new Repository<Synset>(context));
    }

You inject the IAggregatorContext, but not the various repositories your UnitOfWork requires. You should also be ctor injecting all of these dependencies, and using Ninject to bind them. If you can't inject it, you can't mock it, and right now you can't do either with your repositories.

---

Your method of obtaining a database connection string is misguided. If something is hard, it's usually because you're going about it the wrong way. Just use the data context ctor overload that takes in a connection string.

---

I wanted to talk about your using Ninject as a service locator, but I can't even begin to describe how bad ArticleParserProvider is. I don't even know what it is that you're trying to do here. In order to fix this up, you'll need to take a few steps.

1. Replace every instance of new with an instance that gets injected through the constructor. If you need it to be short lived, pass a factory class instead.

   The number of arguments each ctor takes will grow as you go up the chain towards your composition root. That's okay, and in fact, is a Good Thing™.

2. Don't pass anything from Ninject to anything else. Once you get all of your dependencies up and into your composition root, then bind them all with Ninject. Let the IoC do it's magic and recurse down the dependency tree.

Here's the thing about IoC containers... You don't need them. They just make our code near the root easier to read and deal with. Write your code so that it doesn't know that there even is an IoC container. When you're done, you should be able to create and run an instance of your program using any container, even one that we hand write into the composition root.