Prevent returning the same entity with Entity Framework Core

user_9901683 0
Viewed 34

This is my example app scenario:

public class SampleDbContext : DbContext
{
    public DbSet<User> Users { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=ExampleUserDB;Trusted_Connection=True;");
    }
}

public class User
{
    public int Id { get; set; }
    public bool AnalyseInProgress { get; set; }
    public bool Analysed { get; set; }
}

public class UserService
{
    public User GetUser()
    {
        using (var context = new SampleDbContext())
        {
            using (var transaction = context.Database.BeginTransaction())
            {
                var entity = context.Users
                    .Where(x => !x.Analysed)
                    .Where(x => !x.AnalyseInProgress)
                    .FirstOrDefault();

                entity.AnalyseInProgress = true;
                context.SaveChanges();

                return entity;
                transaction.Commit();
            }
        }
    }

    public void MarkUserAsAnalysed(int id)
    {
        using (var context = new SampleDbContext())
        {
            var entity = context.Users
                .Find(id);

            entity.Analysed = true;
            context.SaveChanges();
        }
    }
}

By adding transaction and AnalyseInProgress property, I wanted to prevent returning the same user to multiple applications, if they call UserService.GetUser at the same time.

Unfortunately, it doesn't work as I expected. Sometimes, service returns the same user. Is there any built-in solution to that?

Thanks.

entity-framework-core c#
1 Answers

To ensure that two connections across systems cannot end up assessing the same row as available, you need to employ pessimistic locking on the connection when reading. Note that pessimistic locking when used extensively will lead to performance issues, so your focus should be to only use it where absolutely necessary and for as little time as needed. Get in, get done, get out.

public User GetUser()
{
    User user = null;
    var txOptions = New TransactionOptions {IsolationLevel = System.Transactions.IsolationLevel.Serializable};  
    using(var txScope = new TransactionScope(TransactionScopeOption.Required, txOptions)) 
    {         
        using (var context = new SampleDbContext())
        {
            user = context.Users
                .OrderBy(x => x.CreatedAt)
                .FirstOrDefault(x => !x.Analysed && !x.AnalyseInProgress);
            if(user != null)
            {
                 user.AnalyseInProgress = true;
                 context.SaveChanges();
                 context.Entry(user).State = EntityState.Detached;  
            }
        }
        txScope.Complete();
    }
    return user;
}

So a couple things here. First we use an isolation level of Serializable to ensure that only one operation can read this user row if it is finalized. Multiple calls to GetUser, such as coming from multiple web servers will wait if one of the other running calls is still within that Tx Scope. When returning entities outside of the scope of the DbContext that read them then it is best to ensure that they are marked as Detached. Normally you would read these AsNoTracking but since we want to update that flag on them first, we read the entity as a tracked instance, perform the update, then detach it before returning. In your original code the transaction.Commit() would never run because it was after your return statement. (intellisense would be highlighting that.)

When using FirstOrDefault like that you should specify an OrderBy clause, and you need to handle the possibility that no record is returned. The original code would have thrown a NullReferenceException the minute all unprocessed users was achieved.

I don't recommend passing detached entities around, but rather DTOs or ViewModels to avoid confusion around whether a method accepting an entity is getting a tracked, real entity, or getting a detached, or worse, partially filled entity-like class.

I would also recommend considering using an enumeration or such to represent the User's "Status" or such rather than individual flags like "IsProcessing" "IsProcessed". For instance using an enumeration for "Unprocessed, Processing, Processed" means you can simplify the logic to:

user = context.Users
    .OrderBy(x => x.CreatedAt)
    .FirstOrDefault(x => x.Status == UserStatuses.Unprocessed);

then:

user.Status = UserStatuses.Processing;

It can save accidentally forgetting to append combinations of flags to Where type queries.

user_4234970
Related