2
\$\begingroup\$

I am writting an API backend application using .NET Core and Visual Studio.

Here is the solution structure:

[ProjectName] - Solution

  • [ProjectName].API - API project containing Controllers, DTOs, AutoMapper etc.
  • [ProjectName].Core - Class Library project containing services and business logic
  • [ProjectName].Data - Class Library project containing DbContext and Migrations
  • [ProjectName].Domain - Class Library project containing database entities
  • [ProjectName].Interfaces - Class Library project containing interfaces
  • [ProjectName].Repositories - Class Library project containing repositories
  • [ProjectName].Tests - Console Application containing unit tests

Here is the code structure:

In .Repositories project there is BaseRepository:

public class BaseRepository<T, TPrimaryKey> : IBaseRepository<T, TPrimaryKey> where T : class where TPrimaryKey : struct
{
    private readonly DatabaseContext _dbContext;

    public BaseRepository(DatabaseContext dbContext)
    {
        _dbContext = dbContext;
    }

    public async Task<IEnumerable<T>> GetAll()
    {
        return await _dbContext.Set<T>().ToListAsync();
    }

    public IQueryable<T> GetQueryable()
    {
        return _dbContext.Set<T>();
    }

    public async Task<T> Find(TPrimaryKey id)
    {
        return await _dbContext.Set<T>().FindAsync(id);
    }

    public async Task<T> Add(T entity, bool saveChanges = true)
    {
        await _dbContext.Set<T>().AddAsync(entity);
        if (saveChanges) 
            await _dbContext.SaveChangesAsync();

        return await Task.FromResult(entity);
    }

    public async Task Edit(T entity, bool saveChanges = true)
    {
        _dbContext.Entry(entity).State = EntityState.Modified;
        if (saveChanges) 
            await _dbContext.SaveChangesAsync();
    }

    public async Task Delete(T entity, bool saveChanges = true)
    {
        if (entity == null)
            throw new NullReferenceException();

        _dbContext.Set<T>().Remove(entity);
        if (saveChanges) 
            await _dbContext.SaveChangesAsync();
    }

    public async Task<IEnumerable<T>> BulkInsert(IEnumerable<T> entities, bool saveChanges = true)
    {
        foreach (T entity in entities)
        {
            await _dbContext.Set<T>().AddAsync(entity);
        }

        if (saveChanges) 
            await _dbContext.SaveChangesAsync();

        return await Task.FromResult(entities);
    }

    public async Task BulkUpdate(IEnumerable<T> entities, bool saveChanges = true)
    {
        foreach (T entity in entities)
        {
            _dbContext.Entry(entity).State = EntityState.Modified;
        }

        if (saveChanges) 
            await _dbContext.SaveChangesAsync();
    }

    public async Task Save()
    {
        await _dbContext.SaveChangesAsync();
    }
}

In .Interfaces project there is:

IBaseRepository:

public interface IBaseRepository<T, E> where T : class where E : struct
{
    Task<IEnumerable<T>> GetAll();
    IQueryable<T> GetQueryable();
    Task<T> Find(E id);
    Task<T> Add(T entity, bool saveChanges = true);
    Task Edit(T entity, bool saveChanges = true);
    Task Delete(T entity, bool saveChanges = true);
    Task<IEnumerable<T>> BulkInsert(IEnumerable<T> entities, bool saveC
    Task BulkUpdate(IEnumerable<T> entities, bool saveChanges = true);
    Task Save();
}

IServiceBase:

public interface IServiceBase<TEntity, TPrimaryKey>
{
    Task<TEntity> GetById(TPrimaryKey id);
    Task<TEntity> GetSingle(Expression<Func<TEntity, bool>> whereCondition);
    Task<IEnumerable<TEntity>> GetAll();
    IEnumerable<TEntity> GetAll(Expression<Func<TEntity, bool>> whereCondition);
    IQueryable<TEntity> GetAllQueryable();
    IQueryable<TEntity> Query(Expression<Func<TEntity, bool>> whereCondition);
    Task<TEntity> Create(TEntity entity);
    Task Delete(TEntity entity);
    Task Update(TEntity entity);
    Task<long> Count(Expression<Func<TEntity, bool>> whereCondition);
    Task<long> Count();
    Task<IEnumerable<TEntity>> BulkInsert(IEnumerable<TEntity> entities);
    Task BulkUpdate(IEnumerable<TEntity> entities);
}

and all interfaces of concrete entity services:

for example IAddressServices:

public interface IAddressService : IServiceBase<Address, Guid>
{
    Task<Address> VerifyAddress(Address address);
}

In .Core project there is:

ServiceBase:

public abstract class ServiceBase<TEntity, TRepository, TPrimaryKey> : IServiceBase<TEntity, TPrimaryKey>
        where TEntity : class
        where TPrimaryKey : struct
        where TRepository : IBaseRepository<TEntity, TPrimaryKey>
{
    public TRepository Repository;

    public ServiceBase(IBaseRepository<TEntity, TPrimaryKey> rep)
    {
        Repository = (TRepository)rep;
    }

    public virtual async Task<TEntity> GetById(TPrimaryKey id)
    {
        return await Repository.Find(id);
    }

    public async Task<TEntity> GetSingle(Expression<Func<TEntity, bool>> whereCondition)
    {
        return await Repository.GetQueryable().Where(whereCondition).FirstOrDefaultAsync();
    }

    public async Task<IEnumerable<TEntity>> GetAll()
    {
        return await Repository.GetAll();
    }

    public IEnumerable<TEntity> GetAll(Expression<Func<TEntity, bool>> whereCondition)
    {
        return Repository.GetQueryable().Where(whereCondition);
    }

    public IQueryable<TEntity> GetAllQueryable()
    {
        return Repository.GetQueryable();
    }

    public IQueryable<TEntity> Query(Expression<Func<TEntity, bool>> whereCondition)
    {
        return Repository.GetQueryable().Where(whereCondition);
    }

    public virtual async Task<TEntity> Create(TEntity entity)
    {
        return await Repository.Add(entity);
    }

    public virtual async Task Delete(TEntity entity)
    {
        await Repository.Delete(entity);
    }

    public virtual async Task Update(TEntity entity)
    {
        await Repository.Edit(entity);
    }

    public async Task<long> Count(Expression<Func<TEntity, bool>> whereCondition)
    {
        return await Repository.GetQueryable().Where(whereCondition).CountAsync();
    }

    public async Task<long> Count()
    {
        return await Repository.GetQueryable().CountAsync();
    }       

    public async Task<IEnumerable<TEntity>> BulkInsert(IEnumerable<TEntity> entities)
    {
        return await Repository.BulkInsert(entities);
    }

    public async Task BulkUpdate(IEnumerable<TEntity> entities)
    {
        await Repository.BulkUpdate(entities);
    }
}

and concrete implementations of services:

for example AddressService:

public class AddressService : ServiceBase<Address, IBaseRepository<Address, Guid>, Guid>, IAddressService
{
    public AddressService(IBaseRepository<Address, Guid> rep) : base(rep)
    {
    }

    public async Task<Address> VerifyAddress(Address address)
    {
        //logic
    }
}

In .API project there are controllers:

for example ProductController:

 public class ProductController : ControllerBase
    {
        private readonly IProductService _productService;
        private readonly IAddressService _addressService;
        private readonly ILogger _logger;
        private readonly IMapper _mapper;

        public ProductController (IProductService productService,
            IAddressService addressService,
            ILogger<ProductController> logger,
            IMapper mapper)
        {
            _packageService = packageService;
            _addressService = addressService;
            _logger = logger;
            _mapper = mapper;
        }

        [HttpGet]
        public async Task<IActionResult> GetAllProductsWithAddresses()
        {
            try
            {
                var products = await _productService.GetAllQueryable().Include(x => x.Address).ToListAsync();

                return Ok(_mapper.Map<List<ProductResponse>>(products));
            }
            catch (Exception e)
            {
                _logger.LogError($"An unexpected error occured: ${e}");
                return StatusCode(StatusCodes.Status500InternalServerError);
            }
        }
    }

I have read a lot that Repository pattern is sometimes bad and it should be used only if unit tests are needed. In my case unit tests are needed, so I want to make sure I am using it in correct manner and there won't be performance bottlenecks in the future.

  1. Is using GetQueryable in ProductController correct by Repository pattern design? If not, is it only violation of Repository pattern or there are some other performance issues?
  2. If I wouldn't need unit tests, is it better just to remove Repository layer and use EF directly in services or controllers?
  3. Since I needed to update multiple records in the database, using BulkUpdate from BaseRepository is not perfect in terms of performance, since for updating 1000 rows, 1000 separate queries will be executed in the database. The ideal would be to execute 1 raw SQL query using dbContext.Database.ExecuteSqlCommand. How and where could I integrate execution of raw SQL queries in project setup like this to make sure clean architecture is satisfied?
  4. If answer to 3rd question is you can't integrate raw SQL queries in this architecture to make it clean, should I look at 3rd party libraries that enables those features such as Entity Framework plus (https://entityframework-plus.net/?z=ef-extended). I am not so familiar with this library but as I found out they are executing 1 raw SQL query when updating multiple number of records. Example of integration would be await GetAllQueryable().Where(p => p.Name.Contains("test")).UpdateAsync(p => new Product() { Quantity = 10 });. Is it better to use 3rd party libraries such as EF Plus instead of raw SQL queries?

Any help is appreciated.

\$\endgroup\$
7
  • 1
    \$\begingroup\$ Welcome to Code Review! The current question title, which states your concerns about the code, is too general to be useful here. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How to get the best value out of Code Review: Asking Questions for guidance on writing good question titles. \$\endgroup\$ – Toby Speight Feb 23 at 13:40
  • \$\begingroup\$ @TobySpeight Thanks for the advice! I edited the question a little bit, hopefully title is more meaningful now. \$\endgroup\$ – user238115 Feb 23 at 13:59
  • 2
    \$\begingroup\$ Not really - it's still talking about the concerns with the code, rather than about what purpose it serves. I would edit for you, but I can't work out from the description what the code is supposed to do. If you can write what real-world problem the program solves, then that would be an appropriate title. Did you follow the link I gave? Some more guidance is in How to Ask. \$\endgroup\$ – Toby Speight Feb 23 at 14:05
  • \$\begingroup\$ The program does not solve any real-world problem, lets say it like that. The code I provided is code-infrastructure that I am using in my project, by reading on internet some people say IQueryable is not meant to be used outside of the Repository pattern (BaseRepository class in my case). Not sure how the question should look like, but I have several questions down. Firstly I asked this question on standard StackOverflow, but they suggested to me to post it on codereview forum. @TobySpeight \$\endgroup\$ – user238115 Feb 23 at 14:08
  • 2
    \$\begingroup\$ @user238115: You're conflating the two communities' different focus on what questions they tackle. If you're asking about improvements to your code (as a general review), this is the right place. But this isn't the place to start discussing the repository pattern and practices surrounding it. That question is more for SoftwareEngineering.SE. Different communities tackle different questions/issues. \$\endgroup\$ – Flater Feb 24 at 9:46
1
\$\begingroup\$

DbContext is a Repository and Entity Framework uses Unit of Work pattern to wrap it, so if you need to wrap DbContext with a Repository you will need to narrow it down to more focused scope.

You'll find some arguments about wrapping a repository with another repository (e.g. using repository with EF). Generally, it's not about right or wrong, but it's about avoiding redundancy. Nevertheless, sometimes wrapping big repositories with scoped ones can be beneficial to your work as it would minimize the requirements and add more manageability to your code. You will find some open-source projects have done the same scenario (e.g. ASP.NET Core Identity). So, it depends on your application requirements.

Is using GetQueryable in ProductController correct by Repository pattern design? If not, is it only violation of Repository pattern or there are some other performance issues?

Returning the Set<T> would make the repository looses its job!. As Set<T> it's a repository itself, so do you think it would make sense if your repository returns another repository?. In the other hand, returning an IQueryable<T> interface over IEnumerable<T> interface would vary based on your code scope (where and when you want the query to be executed - (database vs memory)), but definitely it would add more flexibility to the repository consumer.

For questions 3 & 4 :

EF is not ideal for bulk operations up until now (maybe will be better in future versions, you'll never know!), and it's not possible to build your own. However, using 3rd party will save time and efforts, and it'll give you clear and manageable code.

Just need to mention that System.Data.SqlClient has SqlBulkCopy class, which is used for bulk insert operations, old school, but still useful.

Returning to your code

The solution structure needs to be more specific, for instance, [ProjectName].Interfaces layer would contain all possible contracts that the application uses, however, it would be more convenient if divided into several abstraction layers. abstractions layers should be related to each layer, like [ProjectName].API.Abstractions, [ProjectName].Data.Abstractions and so on. This would avoid including unwanted code, and would add more scoped and manageability to your contracts, and also would ease work with DI.

The current [ProjectName].Repositories not sure how you'll use it, but since you'll have an API I assume your application architecture would use the API to query Data and return the results to the consumer. If true, then I don't believe you need Repositories as other layers will call API layer and not the Data layer. So, you'll need to use API layer as a service, in which will eliminate the redundancy that you have in current work.

A few notes on current implementation :

IBaseRepository and IServiceBase :

  • Both serve the same purpose, and almost have the same implementation, so it should be combined into one.
  • GetQueryable should be in a separate interface.
  • BulkUpdate and BulkInsert should be in a separate interface.
  • GetById and Find are similar, use one of them.
  • bool saveChanges = true no need for that, as Save() is already there.
  • T, E the generic naming is not descriptive enough in IBaseRepository<T, E>, while IServiceBase<TEntity, TPrimaryKey> is.
  • Count is unnecessary as you can do it in Linq.
  • Save() should return Task<int> which is the number of changed rows.

The simplified version would be something like this :

public interface IRepository : IDisposal
{
    Task<int> Save();
}

public interface IRepositoryQueryable<TEntity, TKey> : IRepository
    where TEntity : class 
    where TKey : struct
{
    IQueryable<TEntity> GetQueryable();
}

public interface IRepositoryBulk<TEntity, TKey> : IRepository
where TEntity : class 
where TKey : struct
{
    Task<int> BulkInsert(IEnumerable<TEntity> entities); // returns number of inserted entries
    Task<int> BulkUpdate(IEnumerable<TEntity> entities); // returns number of updated entries
}

public interface IRepository<TEntity, TKey> : IRepository
    where TEntity : class 
    where TKey : struct
{
    Task<IEnumerable<TEntity>> GetAll();
    IEnumerable<TEntity> GetAll(Expression<Func<TEntity, bool>> whereCondition);
    Task<TEntity> GetSingle(Expression<Func<TEntity, bool>> whereCondition);
    Task<TEntity> GetByKey(TKey id);
    Task Insert(TEntity entity);
    Task Update(TEntity entity);
    Task Delete(TEntity entity);
}
  • IRepository base interface.
  • IRepositoryQueryable<TEntity, TKey> for IQueryable operations
  • IRepositoryBulk<TEntity, TKey> for bulk operations
  • IRepository<TEntity, TKey> for basic operations.

With that, you have more flexibility to extend and manage your contracts. A use case scenario to show how this would be beneficial, is having an entity that doesn't support Bulk operations, if you use the IBaseRepository then either you will have to implement the BulkInsert and BulkUpdate logic or simply throw NotImplementedException, which is unneeded.

Modifying Current Structure

If you treat the API as a service, then you will simplify your structure and work further. Let's take a quick look on how things would be if we change it.

the new Structure :

  • Project.Data : DbContext and all related work.
  • Project.API : Controllers and AutoMapper work.
  • Project.Abstractions: holds application contracts that would be consumed by consumers (outside Data and API layers).
  • Project.Tests : unit tests.

you can do an abstraction for Data and another one for API, but for simplicity I just did one for the full project for demonstration purpose.

We can make our work much simpler by creating one generic repository class under Data and then use this repository in the base controller.

here is the substituted BaseRepository that would have CUSTOM logic :

public sealed class Repository<TEntity, TKey> : IRepository<TEntity, TKey>, IRepositoryQueryable<TEntity, TKey>, IRepositoryBulk<TEntity, TKey>
    where TEntity : class 
    where TKey : struct
{
    private readonly DatabaseContext _context;

    private readonly DbSet<TEntity> _dbSet; 

    protected RepositoryBase() 
    {
        _context = new DatabaseContext(); 
        _dbSet = _context.Set<TEntity>();
    }
    
    public async Task<IEnumerable<TEntity>> GetAll()
    {
        return await _dbSet.ToListAsync();
    }
    
    public async Task Insert(TEntity entity)
    {
        _dbSet.Add(entity);
    }
    
    // add the rest of your custom code
    
}

Now under API we can create a base controller something like this :

[Route("api/[controller]")]
public abstract class ServiceControllerBase<TEntity, TKey> : ControllerBase
    where TEntity : class 
    where TKey : struct
{
    protected Repository<TEntity, TKey> Repository { get; }
    protected ILogger Logger { get; }
    protected IMapper Mapper { get; }

    protected ServiceControllerBase(ILogger logger, IMapper Mapper)
    {
        Repository = new Repository<TEntity, TKey>();
        Logger = logger;
        Mapper = mapper;
    }
}

then your controllers would be :

public class ProductController : ServiceControllerBase<Product, Guid>
{
    public ProductController (ILogger<ProductController> logger, IMapper mapper)
    : base(logger, mapper)  { }

    [HttpGet]
    public async Task<IActionResult> GetAllProductsWithAddresses()
    {
        try
        {
            
            var products = await Repository.GetAllQueryable().Include(x => x.Address).ToListAsync();            
            return Ok(Mapper.Map<List<ProductResponse>>(products));
        }
        catch (Exception e)
        {
            Logger.LogError($"An unexpected error occured: ${e}");
            return StatusCode(StatusCodes.Status500InternalServerError);
        }
    }
}

Now, at this stage all you are doing is just querying the database from the controller, and return the results to the IActionResult based on the given logic.

This allows your application to be easy to integrate. For instance, if you want to add a web project, then in the web project you only need to reference Project.Abstractions and use HttpClient to call the API layer.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.