N-Layered REST API

Question

I am writing a pretty simple REST service in ASP.NET WebAPI with Entity Framework and I decided to use it as an opportunity to learn N-layered enterprise application architecture with some unit/integration tests coverage. I've spent a few days on reading articles about the topic and that lead me to creating a part of the application. However, it also lead me to confusion about some simple things and I would be really grateful for any hints of what I am doing wrong and how it could be done better (questions at the end).

Note: some of the solutions were based on this articles series.

1st layer - WebAPI Controllers (REST endpoints)

I used a few simple rules for WebAPI layer:

1. 1st layer has no knowledge of how data access is handled.
2. Each controller has a Service class where most of the logic (i.e. data access, validations and business operations) is kept.
3. Each method in Service class is treated either as a query (only returning data) or a transaction (which is a complete action with context.SaveChanges() at the end).
4. POST, PATCH and PUT actions should return a complete, updated object and thus service actions (like Insert) return an updated object.
5. There are two kind of models used in this layer: business entities (used by service layer, but not by data access layer) and REST Models (ViewModels returned as responses and RequestModels incoming in request's body).
6. Logging and exception handling is done in globally registered filters so that controller actions are quite clean.

An example of a controller:

[RoutePrefix("api/departments")]
public class DepartmentsController : BaseApiController
{
    IDepartmentsService _departmentsService;

    protected override void Initialize(HttpControllerContext controllerContext)
    {
        base.Initialize(controllerContext);

        _departmentsService = new DepartmentsService();
    }

    [HttpGet]
    [Route]
    public IHttpActionResult GetList()
    {
        IList<Department> departmentsList = _departmentsService.GetAll();

        return Ok(Mapper.Map<List<DepartmentVM>>(departmentsList));
    }

    [HttpPost]
    [Route]
    public IHttpActionResult Post([FromBody]DepartmentPostRequest postRequest)
    {
        if (postRequest == null)
            return ErrorResponse(HttpStatusCode.BadRequest, Messages.ObjectCannotBeEmpty);

        Department department = Mapper.Map<Department>(postRequest);
        Department insertedDepartment = _departmentsService.Insert(department);

        return Content(HttpStatusCode.Created, Mapper.Map<DepartmentDetailsVM>(insertedDepartment));
    }

    [HttpPatch]
    [Route("{id}")]
    public IHttpActionResult PatchUpdate(int id, [FromBody]DepartmentPatchRequest patchRequest)
    {
        if (patchRequest == null)
            return ErrorResponse(HttpStatusCode.BadRequest, Messages.ObjectCannotBeEmpty);

        Department department = _departmentsService.GetSingle(id);

        if (patchRequest.Name != null) department.Name = patchRequest.Name;
        if (patchRequest.Address != null) department.Address = patchRequest.Address;
        if (patchRequest.Active.HasValue) department.Active = patchRequest.Active.Value;

        Department updatedDepartment = _departmentsService.Update(id, department);

        return Ok(Mapper.Map<DepartmentDetailsVM>(updatedDepartment));
    }
}

2nd layer - Business Services

This layer consists of services (which contain complete business actions), validators and DataEntities-to-BusinessEntities mappers. Service actions communicate with repositories to retrieve/push data, do some logic and return results. Some of the logic (like encryption of sensitive data) is also kept inside Business Entities.

public class DepartmentsService : BaseService, IDepartmentsService
{
    public DepartmentsService()
        :base()
    {
    }

    public DepartmentsService(IUnitOfWork uow)
        :base(uow)
    {
    }

    public List<BusinessModel.Department> GetAll()
    {
        IEnumerable<DBModel.Department> departments = _uow.DepartmentsRepository.GetAll();

        return Mapper.Map<List<BusinessModel.Department>>(departments);
    }

    public BusinessModel.Department GetSingle(int id)
    {
        DBModel.Department deparment = _uow.DepartmentsRepository.GetByID(id);

        if (deparment == null)
            throw new RecordNotFoundException();

        return Mapper.Map<DBModel.Department, BusinessModel.Department>(deparment);
    }

    public BusinessModel.Department Update(int departmentID, BusinessModel.Department department)
    {
        // Validation
        DBModel.Department updatedDepartment = _uow.DepartmentsRepository.GetByID(departmentID);
        if (updatedDepartment == null)
            throw new RecordNotFoundException();

        ValidateAndThrowOnFailure(department, new DepartmentUpdateValidator());

        // Operation
        DateTime operationDateUTC = DateTime.UtcNow;

        if (updatedDepartment.Name != department.Name)
        {
            if (_uow.DepartmentsRepository.DepartmentWithNameExists(department.Name, departmentID))
            {
                throw new RecordConflictException(Messages.DepartmentWithNameAlreadyExists);
            }

            updatedDepartment.Name = department.Name;
        }

        updatedDepartment.Address = department.Address;

        if (updatedDepartment.Active != department.Active)
        {
            _uow.DepartmentsRepository.ChangeDepartmentActivity(
                departmentID, department.Active, operationDateUTC);
        }

        // Commit transaction
        _uow.SaveChanges();

        return Mapper.Map<BusinessModel.Department>(updatedDepartment);
    }
}

3rd layer - Data access layer

This layer consists of model generated by Entity Framework (database first), repositories and UnitOfWork.

A generic (base) repository:

public class GenericRepository<TEntity> where TEntity : class
{
    protected DPSContext _context;
    protected DbSet<TEntity> _dbSet;

    /// <summary>
    /// Public Constructor,initializes privately declared local variables.
    /// </summary>
    /// <param name="context"></param>
    public GenericRepository(DPSContext context)
    {
        this._context = context;
        this._dbSet = context.Set<TEntity>();
    }


    /// <summary>
    /// generic Get method for Entities
    /// </summary>
    /// <returns></returns>
    public virtual IEnumerable<TEntity> Get()
    {
        IQueryable<TEntity> query = _dbSet;
        return query.ToList();
    }

    /// <summary>
    /// Generic get method on the basis of id for Entities.
    /// </summary>
    /// <param name="id"></param>
    /// <returns></returns>
    public virtual TEntity GetByID(object id)
    {
        return _dbSet.Find(id);
    }

    /// <summary>
    /// generic Insert method for the entities
    /// </summary>
    /// <param name="entity"></param>
    public virtual void Insert(TEntity entity)
    {
        _dbSet.Add(entity);
    }

    /// <summary>
    /// Generic Delete method for the entities
    /// </summary>
    /// <param name="id"></param>
    public virtual void Delete(object id)
    {
        TEntity entityToDelete = _dbSet.Find(id);
        Delete(entityToDelete);
    }

    // ... (more generic methods)
}

A derived repository:

public class DepartmentsRepository : GenericRepository<Department>
{
    public DepartmentsRepository(DPSContext context) : base(context)
    {
    }

    public virtual bool DepartmentWithNameExists(string name, int? omitID = null)
    {
        IQueryable<Department> query = _dbSet.Where(d => d.Name == name);

        if (omitID.HasValue)
        {
            query = query.Where(d => d.ID != omitID);
        }

        return query.FirstOrDefault() != null;
    }

    public void ChangeDepartmentActivity(int departmentID, bool newActivity, DateTime operationDateUTC)
    {
        Department department = _dbSet.Find(departmentID);

        department.Active = newActivity;
        department.DepartmentsActivationHistories.Add(new DepartmentsActivationHistory {
            Active = newActivity,
            InsertDateUTC = operationDateUTC
        });
    }

    public int GetClientsCount(int departmentID, bool onlyActive = false)
    {
        var clientsQuery = _context.Clients.Where(c => c.DepartmentID == departmentID);

        if (onlyActive)
            clientsQuery = clientsQuery.Where(c => c.AgreementStatus.ClientActive == true);

        return clientsQuery.Count();
    }

    public int GetInstructorsCount(int departmentID, bool onlyActive = false)
    {
        var instructorsQuery = _context.Instructors.Where(i => i.DepartmentID == departmentID);

        if (onlyActive)
            instructorsQuery = instructorsQuery.Where(i => i.Active);

        return instructorsQuery.Count();
    }
}

Unit tests

I'm new to unit tests so it was rather an attempt than something which I can call a successful unit testing. I created tests (with NUnit and Moq) only for Business Services layer. To give you some overview, I will post some code snippets of the most important parts.

This is what happens at the beginning and before each DepartmentsService test:

#region Variables

    private IDepartmentsService _departmentsService;
    private IUnitOfWork _unitOfWork;
    private List<DBModel.Department> _departments;
    private Mock<DepartmentsRepository> _departmentsRepository;
    private DBModel.DPSContext _dbEntities;

    #endregion

    #region Setup

    [OneTimeSetUp]
    public void Setup()
    {
        _departments = SetUpDepartments();
        Mapper.Initialize(c =>
        {
            DBToBusinessMappings.RegisterUsing(c);
        });
    }

    [SetUp]
    public void ReInitializeTest()
    {
        _departments = SetUpDepartments();
        _dbEntities = new Mock<DBModel.DPSContext>().Object;
        _departmentsRepository = SetUpDepartmentsRepository();
        var unitOfWork = new Mock<IUnitOfWork>();
        unitOfWork.SetupGet(s => s.DepartmentsRepository).Returns(_departmentsRepository.Object);
        _unitOfWork = unitOfWork.Object;
        _departmentsService = new DepartmentsService(_unitOfWork);
    }

In short, DepartmentsRepository (derived from GenericRepository) is mocked. Its most important methods are made to work on local data, for example:

mockRepo.Setup(p => p.GetAll()).Returns(entitiesList);

Then, business layer is tested with the use of mocked repository, for example:

    [Test]
    public void Insert_WhenDepartmentWithNameExists_ThrowConfilctException()
    {
        // Setup
        _departmentsRepository.Setup(m => m.DepartmentWithNameExists(It.IsNotNull<string>(), It.IsAny<int?>()))
            .Returns(true);

        // Given
        BusinessModel.Department newDepartment = DataInitializer.GetExampleOfNewDepartment();

        // Then
        Assert.Throws<RecordConflictException>(() => _departmentsService.Insert(newDepartment));
    }

---

Problems

I found that my separation of Data Access Layer and Business Services layer doesn't work well for every case. I sometimes have problems to decide when to put something in repository or business layer.

Example: if you look at DepartmentsRepository.ChangeDepartmentActivity, it does two things: changes activity and writes activation history to database. On the one hand it's simple CRUD, but on the other it's mixed with logic. I put it in repository, because I wanted it to be accessible across different services, but that's probably wrong. It also complicates testing - if it's in repository, I would need to mock the exact behaviour of this method and test it in another place, which seems like an overkill.

Questions:

1. What should I do with cases like the one above? Add another layer with common business logic? Or put such cases inside one business service and then reference this service in another when needed?
2. When should I invoke context.SaveChanges()? I made an assumption that it should be invoked in service methods. But maybe that's wrong assumption and I should invoke that inside controllers? What advantages/disadvantages does it have?
3. Is Repositories/UoW layer really needed? I sometimes think that it would be easier to treat DbContext as UnitOfWork and use it directly in services layer. But if so, where to put logic which is common to more than one service?
4. And last question, related to unit testing - maybe instead of mocking repositories I should try to mock DbContext? Would it make unit testing easier?

Answer 1

If you use an IDepartmentsService/DepartmentsService, then you should inject this via DI, not initialize it in a constructor.

---

But quite frankly, I'd avoid the endless separating into a Service which then calls an UoW which uses a Repository... To me this creates more problems than it solves.

Instead keep it simple:

• Look at implementing MediatR to move code from the Controllers to Handlers.
• Look into following this advice and use a DbContextScope.
• Validate incoming input via FluentValidation.

If your Handler becomes too complicated, move code to dedicated classes. Same for code you need to re-use -- also consider Extensions.