dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.74k stars 3.18k forks source link

EF not updating shadow properties when calling DB context save changes from Async asp.net core webapi controller. #7259

Closed asadsahi closed 2 years ago

asadsahi commented 7 years ago

I am auditing all my entities using 4 extra shadow properties by implementing IAuditable interface to tell EF to log only those entities which implement IAuditable interface:

Here is my DB context class:

 public class ApplicationDbContext : IdentityDbContext<ApplicationUser, ApplicationRole, int>
    {
        ......................... DbSet poperties here (removed for brevity)

        public ApplicationDbContext(DbContextOptions<ApplicationDbContext> options)
        : base(options)
        {
            Database.EnsureCreated();
        }

        /// <summary>
        /// We overrride OnModelCreating to map the audit properties for every entity marked with the 
        /// IAuditable interface.
        /// </summary>
        /// <param name="modelBuilder"></param>
        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            foreach (var entityType in modelBuilder.Model.GetEntityTypes()
                .Where(e => typeof(IAuditable).IsAssignableFrom(e.ClrType)))
            {
                modelBuilder.Entity(entityType.ClrType)
                    .Property<DateTime>("CreatedAt");

                modelBuilder.Entity(entityType.ClrType)
                    .Property<DateTime>("UpdatedAt");

                modelBuilder.Entity(entityType.ClrType)
                    .Property<string>("CreatedBy");

                modelBuilder.Entity(entityType.ClrType)
                    .Property<string>("UpdatedBy");
            }

            base.OnModelCreating(modelBuilder);
        }

        /// <summary>
        /// Override SaveChanges so we can call the new AuditEntities method.
        /// </summary>
        /// <returns></returns>
        public override int SaveChanges()
        {
            this.AuditEntities();

            return base.SaveChanges();
        }

        /// <summary>
        /// Method that will set the Audit properties for every added or modified Entity marked with the 
        /// IAuditable interface.
        /// </summary>
        private void AuditEntities()
        {

            DateTime now = DateTime.Now;
            // Get the authenticated user name 
            string userName = ......get username from user service

            // For every changed entity marked as IAditable set the values for the audit properties
            foreach (EntityEntry<IAuditable> entry in ChangeTracker.Entries<IAuditable>())
            {
                // If the entity was added.
                if (entry.State == EntityState.Added)
                {
                    entry.Property("CreatedBy").CurrentValue = userName;
                    entry.Property("CreatedAt").CurrentValue = now;
                }
                else if (entry.State == EntityState.Modified) // If the entity was updated
                {
                    entry.Property("UpdatedBy").CurrentValue = userName;
                    entry.Property("UpdatedAt").CurrentValue = now;
                }
            }
        }

From Asp.Net async controller action when I save changes shadow properties aren't logged:

  [HttpPut("{id}")]
        public async Task<IActionResult> PutCompany([FromRoute] int id, [FromBody] Company company)
        {
            if (!ModelState.IsValid)
            {
                return BadRequest(ModelState.GetModelErrors());
            }

            _context.Entry(company).State = EntityState.Modified;

            try
            {
                _context.SaveChanges();
            }
            catch (DbUpdateConcurrencyException)
            {
                if (!CompanyExists(id))
                {
                    return NotFound();
                }
                else
                {
                    throw;
                }
            }

            return Ok(company);
        }

However, synchronous version of action calls save changes and logs the shadow properties correctly.

  [HttpPut("{id}")]
        public IActionResult PutCompany([FromRoute] int id, [FromBody] Company company)
        {
            if (!ModelState.IsValid)
            {
                return BadRequest(ModelState.GetModelErrors());
            }

            _context.Entry(company).State = EntityState.Modified;

            try
            {
                _context.SaveChanges();
            }
            catch (DbUpdateConcurrencyException)
            {
                if (!CompanyExists(id))
                {
                    return NotFound();
                }
                else
                {
                    throw;
                }
            }

            return Ok(company);
        }

Am I missing something in context or during save changes?

Thanks

neridonk commented 7 years ago

dont you need an await?

asadsahi commented 7 years ago

Where?

neridonk commented 7 years ago

await SaveChangesAsync(); and so on

ajcvickers commented 7 years ago

@asadsahi Have you verified that your SaveChanges method is being called? If it is not being called, then check:

asadsahi commented 7 years ago

@ajcvickers Save changes method was being called, but for some reason it wasn't auditing entities. Looking at the code it sounds a little strage, but not sure why it was doing it. After changing to non-async version of controller actions and calling savechanges synchronously too, shadow properties were being updated.

However, got it working now with changing savechange to async as well.

So what works for me is overriding SaveChange to SaveChangesAsync and changing controller actions to async too.

public override Task<int> SaveChangesAsync(CancellationToken cancellationToken = default(CancellationToken))
        {
            this.AuditEntities();

            return base.SaveChangesAsync(cancellationToken);
        }

What doesn't work is calling savechanges synchronously from async controller actions.

I am using IdentityDbContext instead of custom one.

ajcvickers commented 7 years ago

@asadsahi When you say, "Save changes method was being called, but for some reason it wasn't auditing entities." can you explain a bit more what you mean by "it wasn't auditing entities?" Which part of the AuditEntities method was not working? For example, was ChangeTracker.Entries<IAuditable>() returning no values? Or something else?

ajcvickers commented 7 years ago

EF Team Triage: Closing this issue as the requested additional details have not been provided and we have been unable to reproduce it.

andrzejmartyna commented 6 years ago

I've had the same "issue". Overriding SaveChanges/SaveChangesAsync is confusing for me. @ajcvickers, you are absolutely right that the function called should be the same that overridden :)

But, looking into code I see FOUR SaveChanges. int SaveChanges() int SaveChanges(bool acceptAllChangesOnSuccess) Task SaveChangesAsync(bool acceptAllChangesOnSuccess, CancellationToken cancellationToken) Task SaveChangesAsync(CancellationToken cancellationToken)

Which of these should I override? Naive way for me was to override all of them but I've found that they call each other.

I would prefer to have single method to override :)

ajcvickers commented 6 years ago

@andrzejmartyna EF never calls SaveChanges, so really you only need to overrride the method(s) that your code will call. That being said, there are async and sync methods which don't call each other. Within each group one method is just sugar for calling the other method with a default value for the param. So overriding the one with the param would catch all uses.