DI service with proper use of cache and dbContext

Question

I really want to make good, clean, and correct code, so I have a few basic questions. In the beginning I have a service with GetName . _dbContext belongs to another DI service, for example _cache

 public Task<string> GetName(string title) { var articleList = await _cache.GetOrCreate("CacheKey", entry => { entry.SlidingExpiration = TimeSpan.FromSeconds(60 * 60); return _dbContext.Articles.ToListAsync; }); var article = articleList.Where(c => c.Title == title).FirstOrDefault() if(article == null) { return "Non exist" } return article.Name(); } 

• In official docs, we can read that the EF context is not thread safe, so should I add await after returning to GetOrCreate ?
• Would it be appropriate to install it as a Singleton service and use it in razor actions and representations?
• In official docs, we can read: Avoid the "data storage" objects that exist only to allow access to another object in DI services. I worry about this line when I see my service.

Answer 1

The fact that the EF context is not thread safe is not related to async \ await, so your first question doesn't make much sense. In any case, the common best practice is not to reuse a single EF context for multiple logical operations. In multi-threaded applications, violation of this rule usually leads to disaster immediately. If you use the same EF context from the same thread in a while, a violation can lead to certain unexpected behavior, especially if you use the same context for many operations for a long time.

In ASP.NET, each request has a dedicated thread, each request does not last long, and each request usually represents a single logical operation. For this reason, the EF context is usually logged based on life expectancy, which means that each request has a separate context, which is deleted when the request ends. For most applications, this pattern works very well (although you can still get unexpected behavior if you execute complex logic with many database queries in a single HTTP request). I personally still prefer to use a new instance of the EF context for each operation (so in your example, I would introduce the factory EF context and create a new instance inside the body of GetOrCreate , which will delete immediately). However, this is not a very popular opinion.

Thus, each request has a separate context (if it is registered in a limited lifetime), so you should not (in general, if you yourself do not create background threads) worry about multi-threaded access to this instance.

Your method, although still incorrect, because what you are currently storing in the cache is not a List<Article> . Instead, you store Task<List<Acticle>> . It works with the memory cache, because storing in the memory cache is not related to serialization. But as soon as you change your cache provider, everything will break, because Task , obviously, is not serialized. Use GetOrCreateAsync (note that there is still no reason to add await after return ):

 var articleList = await _cache.GetOrCreateAsync("CacheKey", entry => { entry.SlidingExpiration = TimeSpan.FromSeconds(60 * 60); return _dbContext.Articles.ToListAsync(); }); 

Please note: if an element is already present in the cache - asynchronous operation will not be performed, the whole operation will be performed completely synchronously (do not let await fool you - if you are await not doing something, you will need to have some background threads or asynchronous operations).

It is generally not recommended to store EF objects directly in the cache. It is better to convert them to DTO objects and save them. However, if you are saving EF objects - at least remember to turn off the creation of lazy loading and proxies for your context.

It is not correct to register a service such as singleton, because it depends on the EF context, which has a limited lifetime (by default). Register it with the same lifetime that you have for the EF context.

As for the data holder objects, I don’t see how this is related here. Your service has logic (get data from the cache or database) - it does not exist only to access some other objects.

Answer 2

Some argue that DbContext itself is a repository, but some may argue that DbContext follows a work pattern.

For me personally, I like to use a small repository, which basically is a small wrapper around DbContext . Then I expose IDbSet from the repository.

Excuse me for using my GitHub project, as it is easy to explain in real code than in words.

Repository

 public class EfRepository<T> : DbContext, IRepository<T> where T : BaseEntity { private readonly DbContext _context; private IDbSet<T> _entities; public EfRepository(DbContext context) { _context = context; } public virtual IDbSet<T> Entities => _entities ?? (_entities = _context.Set<T>()); public override async Task<int> SaveChangesAsync() { try { return await _context.SaveChangesAsync(); } catch (DbEntityValidationException ex) { var sb = new StringBuilder(); foreach (var validationErrors in ex.EntityValidationErrors) foreach (var validationError in validationErrors.ValidationErrors) sb.AppendLine($"Property: {validationError.PropertyName} Error: {validationError.ErrorMessage}"); throw new Exception(sb.ToString(), ex); } } } public class AppDbContext : DbContext { public AppDbContext(string nameOrConnectionString) : base(nameOrConnectionString) {} public new IDbSet<TEntity> Set<TEntity>() where TEntity : BaseEntity { return base.Set<TEntity>(); } } 

Using the repository in a service

Then I insert the repository into the service classes through the constructor installation.

 public class UserService : IUserService { private readonly IRepository<User> _repository; public UserService(IRepository<User> repository) { _repository = repository; } public async Task<User> GetUserByUserNameAsync(string userName) { var query = _repository.Entities .Where(x => x.UserName == userName); return await query.FirstOrDefaultAsync(); } 

Unit Test Assistant

It's a little tricky to fool asynchronous testing methods. In my case, I use NSubstitute, so I use the following helper method NSubstituteHelper .

If you use moq platforms or other module testing modules, you can read the Entity Framework testing using the Mocking Framework (EF6 onwards) .

 public static IDbSet<T> CreateMockDbSet<T>(IQueryable<T> data = null) where T : class { var mockSet = Substitute.For<MockableDbSetWithExtensions<T>, IQueryable<T>, IDbAsyncEnumerable<T>>(); mockSet.AsNoTracking().Returns(mockSet); if (data != null) { ((IDbAsyncEnumerable<T>)mockSet).GetAsyncEnumerator().Returns(new TestDbAsyncEnumerator<T>(data.GetEnumerator())); ((IQueryable<T>)mockSet).Provider.Returns(new TestDbAsyncQueryProvider<T>(data.Provider)); ((IQueryable<T>)mockSet).Expression.Returns(data.Expression); ((IQueryable<T>)mockSet).ElementType.Returns(data.ElementType); ((IQueryable<T>)mockSet).GetEnumerator().Returns(new TestDbEnumerator<T>(data.GetEnumerator())); } return mockSet; } 

Unit test

 // SetUp var mockSet = NSubstituteHelper.CreateMockDbSet(_users); var mockRepository = Substitute.For<IRepository<User>>(); mockRepository.Entities.Returns(mockSet); _userRepository = mockRepository; [Test] public async Task GetUserByUserName_ValidUserName_Return1User() { var sut = new UserService(_userRepository); var user = await sut.GetUserByUserNameAsync("123456789"); Assert.AreEqual(_user3, user); } 

Return to the original question.

To extract data from the cache, I use this approach. Usually we do not get async when retrieving data from memory.

In official docs, we can read that the EF context is not thread safe, so should I add wait after returning to GetOrCreate?

wait keyword is required for async (not because DbContext is not thread safe)

Would it be appropriate to install it as a Singleton service and use it in the actions and looks of a razor?

It depends on how you use it. However, we should not use a singleton service for request dependency.

For example, I use this singleton object to save settings and configurations that will never change during the life of the application.

In official docs, we can read: Avoid the "data custodian" objects that only exist to allow access to another object in DI services. I worry about this line when I see my service.

You cannot store entire article tables in the cache.

If you store too much data in server memory, the server will eventually throw an Out of Memory Exception. In some cases, it is even slower than querying a single record from the database.

Answer 3

In official docs, we can read that the EF context is not thread safe, so should I add wait after returning to GetOrCreate?

This is not an EF context issue that is not thread safe. When you call async methods you must add await

 public Task<string> GetName(string title) { var articleList = await _cache.GetOrCreateAsync("CacheKey", entry => { entry.SlidingExpiration = TimeSpan.FromSeconds(60 * 60); return await _dbContext.Articles.ToListAsync(); }); var article = await articleList.Where(c => c.Title == title).FirstOrDefaultAsync(); if(article == null) { return "Non exist" } return article.Name(); } 

Would it be appropriate to install it as a Singleton service and use it in the actions and looks of a razor?

I don't think it would be appropriate to set your service as Singleton , because your service depends on _dbContext , which has a Scoped service life. If you register your service as Singleton, then you should enter only Singleton (and _dbContext is obviously not Singleton ... its Scoped). It is probably best to configure the service for the Transient service life. Pay attention to the warning from the main asp.net documents : “The main danger to be wary of is resolving the Scoped service from a singleton. Probably, in this case the service will have an incorrect state when processing subsequent requests.

 public void ConfigureServices(IServiceCollection services) { // ... services.AddTransient<MyService>(); services.AddDbContext<MyDbContext>(ServiceLifetime.Scoped); // ... } 

In official docs, we can read: Avoid the "data custodian" objects that only exist to allow access to another object in DI services. I worry about this line when I see my service.

I understand your concern. When I see GetName , I don’t understand why you cannot just get the first article name from the list of articles from your Controller classes. But then again, this is just a recommendation and should not be taken as a strict rule.