koenbeuk / EntityFrameworkCore.Triggered

Triggers for EFCore. Respond to changes in your DbContext before and after they are committed to the database.
MIT License
554 stars 29 forks source link

The trigger executed twice #155

Closed kangzoel closed 2 years ago

kangzoel commented 2 years ago

Whenever I do insert an entity with its related entity, the trigger is executed twice. I don't think there's a problem in my code though.

CreateStockInputWindow.xaml.cs

...
                    dbContext.StockInputLogs.Add(
                        new StockInputLog()
                        {
                            UserId = Session.Instance.User!.Id,
                            Amount = inputAmount,
                            Stock = new Stock()
                            {
                                ExpirationDate = expirationDate,
                                ItemId = Item!.Id,
                                Amount = inputAmount,
                            }
                        }
                    );
...

StockInputLog.cs

namespace WarehouseApp.Models;

public class StockInputLog : BaseModel
{
    public int StockId { get; set; }

    public int UserId { get; set; }

    public int Amount { get; set; }

    public Stock? Stock { get; set; }

    public User? User { get; set; }
}

Stock.cs

using System;
using System.Collections.Generic;

namespace WarehouseApp.Models;

public class Stock : BaseModel
{
    public DateTime ExpirationDate { get; set; }

    public int ItemId { get; set; }

    public int Amount { get; set; }

    public Item? Item { get; set; }

    public List<StockInputLog>? StockInputLogs { get; set; }

    public List<StockOutputLog>? StockOutputLogs { get; set; }
}

The current workaround is to insert an entity, then call dbContext.SaveChanges(), and finally insert the related entity by selecting the previous entity's id and do dbContext.SaveChanges()

Example.cs

        dbContext.AddRange(
            new User(username: "admin", name: "Admin", password: "Password", isAdmin: true),
            new User(username: "user", name: "User", password: "Password", isAdmin: false)
        );

        dbContext.AddRange(
            new Item(barcode: "8991038775416", name: "Lorem ipsum ", quantifier: "dus"),
            new Item(
                barcode: "8991038775417",
                name: "Dolor sit amet",
                quantifier: "kg",
                totalAmount: 0
            ),
            new Item(
                barcode: "8991038775418",
                name: "Consectetur adipiscing",
                quantifier: "m",
                totalAmount: 0
            )
        );

        dbContext.SaveChanges();

        dbContext.Add(new Stock() { ItemId = 1, ExpirationDate = DateTime.Now.AddDays(10) });

        dbContext.SaveChanges();

        dbContext.AddRange(
            new StockInputLog() { Amount = 10, UserId = 1, StockId = 1 },
            new StockInputLog() { Amount = 20, UserId = 1, StockId = 1 },
            new StockInputLog() { Amount = 15, UserId = 1, StockId = 1 }
        );

        dbContext.SaveChanges();

But it looks ugly. Can anyone help to fix this problem? Thank you in advance

koenbeuk commented 2 years ago

Can you clarify what trigger ran twice and on what entity? Mind that modifying a related Entity within a BeforeSave trigger will cause BeforeSave triggers for that related entity to fire again.

kangzoel commented 2 years ago

This is the trigger I applied

StockInputLogTrigger.cs

internal class StockInputLogTrigger : IBeforeSaveTrigger<StockInputLog>
{
    public Task BeforeSave(
        ITriggerContext<StockInputLog> context,
        CancellationToken cancellationToken
    )
    {
        var stock = context.Entity.Stock;

        switch (context.ChangeType)
        {
            case ChangeType.Added:
                stock!.Amount += context.Entity.Amount;
                break;
            case ChangeType.Deleted:
                stock!.Amount -= context.Entity.Amount;
                break;
        }

        return Task.CompletedTask;
    }
}

I expect it to make new Stock with the Stock.Amount equals to the inserted StockInputLogs.Amount . But instead, it is doubling it. For example, StockInputLogs with the amount of 10 will result in the Stock.Amount of 20

It does work normal when doing normal insert though. Not nested insert like the given example above: CreateStockInputWindow.xaml.cs

koenbeuk commented 2 years ago

@kangzoel, The trigger is not supposed to be fired twice for the same entity/ChangeType combo. What is possible is that the trigger is both registered with the application DI container as well as with the Trigger service, e.g.:

services.AddDbContext(options => {
  options.UseTriggers(triggerOptions => {
    triggerOptions.AddTrigger<StockInputLogTrigger>(); // <-- Here we register the trigger with the trigger service (preferred method).
  });
});

services.AddScoped<StockInputLogTrigger>(); // <-- Here we register the trigger with the DI container.

With the above setup, there is a possibility that the trigger will be fired twice for the same entity/ChangeType combo. (I'm planning on stripping support for triggers registered with the DI container completely in the next major release).

If the above does not help: Please try and are a reproducible.

Meanwhile, I trigger can keep state. Registering a trigger with a Scoped lifetime will ensure that it's state is bound to the lifetime of the DbContext instance. You can use this as a work around for your issue, this would look something like:

triggerOptions.AddTrigger<StockInputLogTrigger>(ServiceLifetime.Scoped); // Register your trigger with a scoped lifetime

class StockInputLogTrigger : IBeforeSaveTrigger<StockInputLog>, IBeforeSaveCompletedTrigger {
  readonly HashSet<StockInputLog> _processedEntities = new();

  public Task BeforeSave(ITriggerContext<StockInputLogTrigger> context, CancellationToken _) {
    if (!_processedEntities.Contains(context.Entity)) { // Ensure that we did not process this entity before
      _processedEntities.Add(context.Entity); // Record it
      // Do your logic
    }
  }

  public Task BeforeSaveCompleted(CacellationToken _) {
    _processedEntities.Clear(); // Optional cleanup in case we expect subsequent calls to SaveChanges within the lifetime of this dbcontext
  }
}
kangzoel commented 2 years ago

I don't use dependency injection. I modified my code but it has not effect. The trigger still executed twice. These are my new codes:

DatabaseContext.cs

using System.Diagnostics;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.DependencyInjection;
using WarehouseApp.Models;

namespace WarehouseApp.Database;

public class DatabaseContext : DbContext
{
    public DbSet<Item> Items => Set<Item>();
    public DbSet<Stock> Stocks => Set<Stock>();
    public DbSet<StockInputLog> StockInputLogs => Set<StockInputLog>();
    public DbSet<StockOutputLog> StockOutputLogs => Set<StockOutputLog>();
    public DbSet<User> Users => Set<User>();

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder
            .UseSqlite("Data Source=database.sqlite")
            .EnableSensitiveDataLogging()
            .LogTo((e) => Trace.WriteLine(e))
            .UseTriggers(triggerOptions =>
            {
                triggerOptions.AddTrigger<Triggers.StockInputLogTrigger>(ServiceLifetime.Scoped);
            });
    }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<Stock>().HasAlternateKey(s => new { s.ItemId, s.ExpirationDate });
    }
}

StockInputLogTrigger.cs

using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using EntityFrameworkCore.Triggered;
using EntityFrameworkCore.Triggered.Lifecycles;
using WarehouseApp.Models;

namespace WarehouseApp.Triggers;

internal class StockInputLogTrigger : IBeforeSaveTrigger<StockInputLog>, IBeforeSaveCompletedTrigger
{
    readonly HashSet<StockInputLog> _processedEntities = new();

    public Task BeforeSave(
        ITriggerContext<StockInputLog> context,
        CancellationToken cancellationToken
    )
    {
        if (!_processedEntities.Contains(context.Entity))
        {
            _processedEntities.Add(context.Entity);

            var stock = context.Entity.Stock;

            switch (context.ChangeType)
            {
                case ChangeType.Added:
                    stock!.Amount += context.Entity.Amount;
                    break;
                case ChangeType.Deleted:
                    stock!.Amount -= context.Entity.Amount;
                    break;
            }
        }

        return Task.CompletedTask;
    }

    public Task BeforeSaveCompleted(CancellationToken _)
    {
        _processedEntities.Clear();

        return Task.CompletedTask;
    }
}

I think I'll use my workaround instead, for the time being, by inserting entities independently (no nesting entity), until the problem is fixed

kangzoel commented 2 years ago

My bad, the problem is in my code, not the library. I shouldn't have initialize the stock value, and let the StockInputLog trigger handle the initial value. Thus the code should be

CreateStockInputWindow.xaml.cs

...
                    dbContext.StockInputLogs.Add(
                        new StockInputLog()
                        {
                            UserId = Session.Instance.User!.Id,
                            Amount = inputAmount,
                            Stock = new Stock()
                            {
                                ExpirationDate = expirationDate,
                                ItemId = Item!.Id,
                                // Removed: Amount = inputAmount,
                            }
                        }
                    );
...