Feedback and Advice on my N-Tier Application Architecture

Question

I am working on an application that will use an N-Tiered approach. I would appreciate any feedback and advice on my architecture before I proceed any further.

DataLayer (Class Library Project)

Context.cs

namespace TimeTracker.DataLayer
{
    public class Context : DbContext
    {
        public Context() : base("Data")
        {
        }

        public DbSet<User> Users { get; set; }
        public DbSet<TimeEntry> TimeEntries { get; set; }
    }
}

TimeEntry.cs

User.cs

Framework (Class Library Project)

DbContextManager.cs

namespace TimeTracker.Framework.DbContextManagement
{
    using System.Data.Entity;

    /// <summary>
    /// Abstract base class for all other DbContextManager classes.
    /// </summary>
    public abstract class DbContextManager
    {
        /// <summary>
        /// Returns a reference to an DbContext instance.
        /// </summary>
        /// <typeparam name="TDbContext">The type of the db context.</typeparam>
        /// <returns>The current DbContext</returns>
        public abstract TDbContext GetDbContext<TDbContext>()
            where TDbContext : DbContext, new();
    }
}

DbContextScope.cs

namespace TimeTracker.Framework.DbContextManagement
{
    using System;
    using System.Collections.Generic;
    using System.Data.Common;
    using System.Data.Entity;
    using System.Data.Entity.Infrastructure;
    using System.Linq;
    using System.Threading;

    /// <summary>
    /// Defines a scope wherein only one DbContext instance is created, and shared by all of those who use it. 
    /// </summary>
    /// <remarks>Instances of this class are supposed to be used in a using() statement.</remarks>
    public class DbContextScope : IDisposable
    {
        /// <summary>
        /// List of current DbContexts (supports multiple contexts).
        /// </summary>
        private readonly List<DbContext> contextList;

        /// <summary>
        /// DbContext scope definitiion.
        /// </summary>
        [ThreadStatic]
        private static DbContextScope currentScope;

        /// <summary>
        /// Holds a value indicating whether the context is disposed or not.
        /// </summary>
        private bool isDisposed;

        /// <summary>
        /// Initializes a new instance of the <see cref="DbContextScope"/> class.
        /// </summary>
        /// <param name="saveAllChangesAtEndOfScope">if set to <c>true</c> [save all changes at end of scope].</param>
        protected DbContextScope(bool saveAllChangesAtEndOfScope)
        {
            if (currentScope != null && !currentScope.isDisposed)
            {
                throw new InvalidOperationException("DbContextScope instances cannot be nested.");
            }

            this.SaveAllChangesAtEndOfScope = saveAllChangesAtEndOfScope;

            this.contextList = new List<DbContext>();

            this.isDisposed = false;

            Thread.BeginThreadAffinity();

            currentScope = this;
        }

        /// <summary>
        /// Gets or sets a value indicating whether to automatically save all object changes at end of the scope.
        /// </summary>
        /// <value><c>true</c> if [save all changes at end of scope]; otherwise, <c>false</c>.</value>
        private bool SaveAllChangesAtEndOfScope { get; set; }

        /// <summary>
        /// Save all object changes to the underlying datastore.
        /// </summary>
        public void SaveAllChanges()
        {
            var transactions = new List<DbTransaction>();

            foreach (var context in this.contextList
                .Select(dbcontext => ((IObjectContextAdapter)dbcontext)
                    .ObjectContext))
            {
                context.Connection.Open();

                var databaseTransaction = context.Connection.BeginTransaction();

                transactions.Add(databaseTransaction);

                try
                {
                    context.SaveChanges();
                }
                catch
                {
                    /* Rollback & dispose all transactions: */
                    foreach (var transaction in transactions)
                    {
                        try
                        {
                            transaction.Rollback();
                        }
                        catch
                        {
                            // "Empty general catch clause suppresses any errors."
                            // Haven't quite figured out what to do here yet.
                        }
                        finally
                        {
                            databaseTransaction.Dispose();
                        }
                    }

                    transactions.Clear();

                    throw;
                }
            }

            try
            {
                /* Commit all complete transactions: */
                foreach (var completeTransaction in transactions)
                {
                    completeTransaction.Commit();
                }
            }
            finally
            {
                /* Dispose all transactions: */
                foreach (var transaction in transactions)
                {
                    transaction.Dispose();
                }

                transactions.Clear();

                /* Close all open connections: */
                foreach (var context in this.contextList
                    .Select(dbcontext => ((IObjectContextAdapter)dbcontext).ObjectContext)
                    .Where(context => context.Connection.State != System.Data.ConnectionState.Closed))
                {
                    context.Connection.Close();
                }
            }
        }

        /// <summary>
        /// Disposes the DbContext.
        /// </summary>
        public void Dispose()
        {
            // Monitor for possible future bugfix.
            // CA1063 : Microsoft.Design : Provide an overridable implementation of Dispose(bool) 
            // on 'DbContextScope' or mark the type as sealed. A call to Dispose(false) should 
            // only clean up native resources. A call to Dispose(true) should clean up both managed 
            // and native resources.
            if (this.isDisposed)
            {
                return;
            }

            // Monitor for possible future bugfix.
            // CA1063 : Microsoft.Design : Modify 'DbContextScope.Dispose()' so that it calls 
            // Dispose(true), then calls GC.SuppressFinalize on the current object instance 
            // ('this' or 'Me' in Visual Basic), and then returns.
            currentScope = null;

            Thread.EndThreadAffinity();

            try
            {
                if (this.SaveAllChangesAtEndOfScope && this.contextList.Count > 0)
                {
                    this.SaveAllChanges();
                }
            }
            finally
            {
                foreach (var context in this.contextList)
                {
                    try
                    {
                        context.Dispose();
                    }
                    catch (ObjectDisposedException)
                    {
                        // Monitor for possible future bugfix.
                        // CA2202 : Microsoft.Usage : Object 'databaseTransaction' can be disposed 
                        // more than once in method 'DbContextScope.SaveAllChanges()'. 
                        // To avoid generating a System.ObjectDisposedException you should not call 
                        // Dispose more than one time on an object.
                    }
                }

                this.isDisposed = true;
            }
        }

        /// <summary>
        /// Returns a reference to a DbContext of a specific type that is - or will be -
        /// created for the current scope. If no scope currently exists, null is returned.
        /// </summary>
        /// <typeparam name="TDbContext">The type of the db context.</typeparam>
        /// <returns>The current DbContext</returns>
        protected internal static TDbContext GetCurrentDbContext<TDbContext>()
            where TDbContext : DbContext, new()
        {
            if (currentScope == null)
            {
                return null;
            }

            var contextOfType = currentScope.contextList
                .OfType<TDbContext>()
                .FirstOrDefault();

            if (contextOfType == null)
            {
                contextOfType = new TDbContext();

                currentScope.contextList.Add(contextOfType);
            }

            return contextOfType;
        }
    }
}

FacadeBase.cs

namespace TimeTracker.Framework.DbContextManagement
{
    using System;
    using System.Collections;
    using System.Collections.Generic;
    using System.Configuration;
    using System.Data.Entity;
    using System.Reflection;

    /// <summary>
    /// Generic base class for all other TimeTracker.BusinessLayer.Facade classes.
    /// </summary>
    /// <typeparam name="TDbContext">The type of the db context.</typeparam>
    /// <typeparam name="TEntity">The type of the entity.</typeparam>
    /// <typeparam name="TEntityKey">The type of the entity key.</typeparam>
    /// <remarks></remarks>
    public abstract class FacadeBase<TDbContext, TEntity, TEntityKey>
        where TDbContext : DbContext, new()
        where TEntity : class
    {
        /// <summary>
        /// Gets the db context.
        /// </summary>
        public TDbContext DbContext
        {
            get
            {
                if (DbContextManager == null)
                {
                    this.InstantiateDbContextManager();
                }

                return DbContextManager != null
                    ? DbContextManager.GetDbContext<TDbContext>()
                    : null;
            }
        }

        /// <summary>
        /// Gets or sets the DbContextManager.
        /// </summary>
        /// <value>The DbContextManager.</value>
        private DbContextManager DbContextManager { get; set; }

        /// <summary>
        /// Adds a new entity object to the context.
        /// </summary>
        /// <param name="newObject">A new object.</param>
        public virtual void Add(TEntity newObject)
        {
            this.DbContext.Set<TEntity>().Add(newObject);
        }

        /// <summary>
        /// Deletes an entity object.
        /// </summary>
        /// <param name="obsoleteObject">An obsolete object.</param>
        public virtual void Delete(TEntity obsoleteObject)
        {
            this.DbContext.Set<TEntity>().Remove(obsoleteObject);
        }

        /// <summary>
        /// Gets all entities for the given type.
        /// </summary>
        /// <returns>DbContext Set of TEntity.</returns>
        public virtual IEnumerable<TEntity> GetAll()
        {
            return this.DbContext.Set<TEntity>();
        }

        /// <summary>
        /// Gets the entity by the specified entity key.
        /// </summary>
        /// <param name="entityKey">The entity key.</param>
        /// <returns>Entity matching specified entity key or null if not found.</returns>
        public virtual TEntity GetByKey(TEntityKey entityKey)
        {
            return this.DbContext.Set<TEntity>().Find(entityKey);
        }

        /// <summary>
        /// Deletes the entity for the specified entity key.
        /// </summary>
        /// <param name="entityKey">The entity key.</param>
        public virtual void DeleteByKey(TEntityKey entityKey)
        {
            var entity = this.DbContext.Set<TEntity>().Find(entityKey);

            if (entity != null)
            {
                this.DbContext.Set<TEntity>().Remove(entity);
            }
        }

        /// <summary>
        /// Instantiates a new DbContextManager based on application configuration settings.
        /// </summary>
        private void InstantiateDbContextManager()
        {
            /* Retrieve DbContextManager configuration settings: */
            var contextManagerConfiguration = ConfigurationManager.GetSection("DbContext") as Hashtable;

            if (contextManagerConfiguration == null)
            {
                throw new ConfigurationErrorsException("A TimeTracker.BusinessLayer.Facade.DbContext tag or its managerType attribute is missing in the configuration.");
            }

            if (!contextManagerConfiguration.ContainsKey("managerType"))
            {
                throw new ConfigurationErrorsException("dbManagerConfiguration does not contain key 'managerType'.");
            }

            var managerTypeName = contextManagerConfiguration["managerType"] as string;

            if (string.IsNullOrEmpty(managerTypeName))
            {
                throw new ConfigurationErrorsException("The managerType attribute is empty.");
            }

            managerTypeName = managerTypeName.Trim().ToUpperInvariant();

            try
            {
                /* Try to create a type based on it's name: */
                var frameworkAssembly = Assembly.GetAssembly(typeof(DbContextManager));

                var managerType = frameworkAssembly.GetType(managerTypeName, true, true);

                /* Try to create a new instance of the specified DbContextManager type: */
                this.DbContextManager = Activator.CreateInstance(managerType) as DbContextManager;
            }
            catch (Exception e)
            {
                throw new ConfigurationErrorsException("The managerType specified in the configuration is not valid.", e);
            }
        }
    }
}

ScopedDbContextManager.cs

namespace TimeTracker.Framework.DbContextManagement
{
    using System.Collections.Generic;
    using System.Data.Entity;
    using System.Linq;

    /// <summary>
    /// Manages multiple db contexts.
    /// </summary>
    public sealed class ScopedDbContextManager : DbContextManager
    {
        /// <summary>
        /// List of Object Contexts.
        /// </summary>
        private List<DbContext> contextList;

        /// <summary>
        /// Returns the DbContext instance that belongs to the current DbContextScope.
        /// If currently no DbContextScope exists, a local instance of an DbContext
        /// class is returned.
        /// </summary>
        /// <typeparam name="TDbContext">The type of the db context.</typeparam>
        /// <returns>Current scoped DbContext.</returns>
        public override TDbContext GetDbContext<TDbContext>()
        {
            var currentDbContext = DbContextScope.GetCurrentDbContext<TDbContext>();

            if (currentDbContext != null)
            {
                return currentDbContext;
            }

            if (this.contextList == null)
            {
                this.contextList = new List<DbContext>();
            }

            currentDbContext = this.contextList.OfType<TDbContext>().FirstOrDefault();

            if (currentDbContext == null)
            {
                currentDbContext = new TDbContext();

                this.contextList.Add(currentDbContext);
            }

            return currentDbContext;
        }
    }
}

UnitOfWorkScope.cs

namespace TimeTracker.Framework.DbContextManagement
{
    /// <summary>
    /// Defines a scope for a business transaction. At the end of the scope all object changes can be persisted to the underlying datastore. 
    /// </summary>
    /// <remarks>Instances of this class are supposed to be used in a using() statement.</remarks>
    public sealed class UnitOfWorkScope : DbContextScope
    {
        /// <summary>
        /// Initializes a new instance of the <see cref="UnitOfWorkScope"/> class.
        /// </summary>
        /// <param name="saveAllChangesAtEndOfScope">if set to <c>true</c> [save all changes at end of scope].</param>
        public UnitOfWorkScope(bool saveAllChangesAtEndOfScope)
            : base(saveAllChangesAtEndOfScope)
        {
        }
    }
}

BusinessLayer (Class Library Project)

TimeEntryFacade.cs

namespace TimeTracker.BusinessLayer.Facade
{
    using System.Collections.Generic;
    using System.Linq;
    using TimeTracker.DataLayer;
    using TimeTracker.DataLayer.Entities;
    using TimeTracker.Framework.DbContextManagement;
    using System;
    using TimeTracker.Framework.AppException;

    public class TimeEntryFacade : FacadeBase<Context, TimeEntry, int>
    {
        public override IEnumerable<TimeEntry> GetAll()
        {
            return base.GetAll()
                .Distinct()
                .ToList();
        }

        /// <summary>
        /// Returns a TimeEntry based on its ID.
        /// </summary>
        /// <param name="productID">ID of an item.</param>
        /// <returns>A TimeEntry object, or null if it does not exist.</returns>
        public TimeEntry GetTimeEntryByID(Guid id)
        {
            return this.DbContext.TimeEntries.FirstOrDefault(x => x.UserID.Equals(id));
        }

        private TimeEntry GetLastTimeEntry(int UserID)
        {
            return this.DbContext.TimeEntries
            .OrderByDescending(x => x.Time)
            .FirstOrDefault(x => x.UserID == UserID);
        }
    }
}

My first issue is how to best work with defining the scope of my business transactions. Currently, if I am going to add, delete or modify an object I need to define my scope by writing my using statement.

using (new UnitOfWorkScope(true))
{
    UserFacade userFacade = new UserFacade();
    User user = userFacade.GetUserByID(1);
    user.LastName = "New Last Name";
}

What can I do to eliminate the need to constantly write out this using statement throughout my application?

Lastly, where would the most appropriate location be to place code like the following?

public void ClockIn(string clockNumber)
        {
            UserFacade userFacade = new UserFacade();
            User user = userFacade.GetUserByClockNumber(clockNumber);
            if (user == null)
                throw new InvalidClockNumberException();

            TimeEntryFacade timeEntryFacade = new TimeEntryFacade();
            TimeEntry lastTimeEntry = timeEntryFacade.GetLastTimeEntry(user.UserID);
            if (lastTimeEntry.Action == TimeEntryAction.ClockIn)
                throw new NotClockedOutException(user.UserID);

            TimeEntry clockIn = new TimeEntry()
                {
                    TimeEntryID = Guid.NewGuid(),
                    UserID = user.UserID,
                    Time = DateTime.Now,
                    Action = TimeEntryAction.ClockIn
                };

            timeEntryFacade.Add(clockIn);
        }

I currently have this in my UI layer which I realize is a mistake as I should be able to replace my UI without having to disrupt common code like this.

Answer 1

I like your effort, there is a separation of responsibilities in what you present

Here are my remarks:

I am working on an application that will use an N-Tiered approach.

While many people use the terms N-Tiered and N-Layered interchangeably, there is a fundamental difference between the two. The former refers to separate services spanning process or host boundaries while the latter refers to the logical seperation in code structure.

Your example looks more like a logical separation than a physical seperation, therefore the term layered would be more appropriate. It would be N-Tiered for example if you deployed your datalayer as a standalone service that would be invoked via remoting or web services. As far as I can see, it is going to live within the same application domain as the caller, invoked via method calls. See Multilayered Architechture vs MultiTiered Architecture

What can I do to eliminate the need to constantly write out this using statement throughout my application?

You could use Aspect Oriented Programming for such a cross-cutting concern, for example you would create an aspect along the following lines:

[Serializable]
[MulticastAttributeUsage(MulticastTargets.Method)]
public class UnitOfWorkScopeAspect: PostSharp.Aspects.OnMethodBoundaryAspect
{
    [NonSerialized]
    UnitOfWorkScope _UnitOfWorkScope;

    // Invoked at runtime before that target method is invoked.
    public override void OnEntry(PostSharp.Aspects.MethodExecutionArgs args)
    {
        _UnitOfWorkScope = new UnitOfWorkScope(true);
    }

    // Invoked at runtime after the target method is invoked (in a finally block).
    public override void OnExit(PostSharp.Aspects.MethodExecutionArgs args)
    {
        _UnitOfWorkScope.Dispose();
    }
}

You then use your newly created Aspect to decorate the methods that should use the unit of work pattern. eg:

[UnitOfWorkScope]
public User user CarryOutScopedLogic()
{
    UserFacade userFacade = new UserFacade();
    User user = userFacade.GetUserByID(1);
    user.LastName = "New Last Name";
    return user; 
}

You can imagine the above as being replaced with:

OnEntry(); // <- This is where we create a new UnitOfWorkScope
try
{
   // Method body for CarryOutScopedLogic
   OnSuccess();
}
catch ( Exception e )
{
   OnException();
}
finally
{
  OnExit();  // <- This is where we dispose our new UnitOfWorkScope
}

This is just a sample using PostSharp, but there are plenty more aop libraries out there.

Some further considerations on the above: if you need to call many different methods that require their own unit of work logic, you could use a ThreadStatic instance of your UnitOfWorkScope within the aspect and check if it does not already exist then create it as well as keeping track of how many times a UnitOfWork has been entered (UnitOfWorkEntryCount) so that you only dispose of it if the UnitOfWorkEntryCount is zero.

where would the most appropriate location be to place code like the following?

public void ClockIn(string clockNumber)

Your hunch is right, that function shouldn't live side by side a buttonclick event for example. You could split out your UI logic following one flavor of MVP/MVVM or MVC - But even so, for this particular example, such orchestrating logic would live in a business logic class of it's own.

The most likely candidate for this following your architecture would seem to be the EmployeeFacade (or UserFacade) as it is the Employee that clocks in by orchestrating the dependencies accordingly.