Onion Architecture - Service Layer Responsibility

Question

I am learning Onion Architecture by Jeffrey Palermo for more than 2 weeks now. I have created a test project by following this tutorial. While studying I came across this question on SO. According to accepted answer, one person nwang suggests that Methods like GetProductsByCategoryId should not be in Repository and one the other hand Dennis Traub suggests that it is the responsibility of the Repository. What I am doing is :

I have a General Repository in Domain.Interface in which I have a method Find :

public interface IRepository<TEntity> where TEntity : class
{
     IEnumerable<TEntity> Find(Expression<Func<TEntity, bool>> filter = null);
     .......
     .......
     .......
}

Then I created a BaseRepository in Infrastucture.Data:

public class RepositoryBase<TEntity> : IRepository<TEntity> where TEntity : class
{
     internal readonly DbSet<TEntity> dbSet;
     public virtual IEnumerable<TEntity> Find(
            Expression<Func<TEntity, bool>> filter = null)
     {
            IQueryable<TEntity> query = dbSet;

            if (filter != null)
            {
                query = query.Where(filter);
            }
            return query.ToList();
     }
}

And I have a concrete repository in Infrastructure.Data

public class ProductRepository : RepositoryBase<Product>, IProductRepository
{
      public ProductRepository(MyDBContext context)
           : base(context)
       {         

       }
}

Now what I am doing in my Service Layer is Injecting Repository into Service and calling Repository.Find for methods like GetProductsByCategoryId. Like :

public class ProductService : IProductService
{
     private readonly IUnitOfWork _unitOfWork;
     private readonly IProductRepository _productRepository;

     public ProductService(IUnitOfWork unitOfWork, IProductRepository productRepository)
     {
          _unitOfWork = unitOfWork;
          _productRepository = productRepository;
     }

     public IList<Product> GetProductsByCategoryId(int CategoryId)
     {
          // At the moment, My code is like this:
          return _productRepository.Find(e => e.CategoryId == CategoryId).ToList();

          // My confusion is here. Am I doing it right or I need to take this code to 
          // ProductRepository and call _productRepositoy.GetProductsByCategoryId(CategoryId) here instead.
          // If I do this, then Service Layer will become more of a wrapper around repository. Isn't it?
          // My question is : What exactly will be the responsibility of the Service Layer in Onion Architecture?
         }
    }

Answer 1

The way you designed your application is ok... but only if your service will come to handle other things than just wrap up the repository methods!

Always keep in mind the YAGNI principle that says:

Always implement things when you actually need them, never when you just foresee that you need them

Let's say that you have a user story that says that whenever a product description is not found in your DB, you should retreive it from somewhere else (calling an external service or something). Then it seems obvious that your ProductService will have to have a

private readonly IProductRepository _productRepository;

but also a

private readonly IProductDescriptionService _productDescriptionService;

In that situation it really makes sense to add a service layer on top of your repositories.

Answer 2

I find that sometimes things can get over abstracted for the sake of it and offer no real value. I would say that the structure in your example is fine and follows the pattern correctly. Your service layer, correctly, is acting to serve the needs of the client UI, it is loosely coupled to the data layer and contains any business logic needed to manipulate the data.

I always think it is more productive to start simple and build upon your structure than it is to over abstract, over complicate and over bloat a project. A business or technical case will often drive the project, and dictate whether it is needed.