Sqlite migrations for concurrency tokens

[Virmak]

Problem

Since Sqlite doesn't support database-generated concurrency tokens, and using application-managed concurrency tokens with Sqlite like suggested in the docs doesn't throw an [DbUpdateConcurrencyException] An "AFTER UPDATE" trigger can be used to implement a concurrency token in EF Core with Sqlite. The only down side of this solution, is the need to manually modify/create migrations to create and drop the trigger from the database.

Solution

Automatically generate migration operations to create/drop a trigger from the database. I already wrote a small library to do this, but it's using some internal EF core APIs and I thought it might be a good idea to include this in EF core Sqlite distribution.

Here's a link to a solution (WIP): https://github.com/Virmak/ef-core-sqlite-concurrency/tree/dev

I opened this issue to ask if there was a reason EF core Sqlite doesn't already generate migrations to create/drop a trigger to implement concurrency tokens automatically?

If you think it's a good idea to have this feature I can fork the repo and start working on a PR.

[ajcvickers]

Note for team: historically, our proposed solution for concurrency is an offline lock--see #2195. This is a different option. There are pros and cons.

[roji]

How does this solution work, given that EF needs to be fetch back the updated concurrency token (using the RETURNING clause), but changes done by triggers aren't taken into account?

From the docs:

The RETURNING clause only returns rows that are directly modified by the DELETE, INSERT, or UPDATE statement. The RETURNING clause does not report any additional database changes caused by foreign key constraints or triggers.

[Virmak]

@roji Thanks for the response I wasn't aware that EF needed to fetch back the updated concurrency token as I'm fairly new to this codebase. This solution works because EF will throw a DbUpdateConcurrencyException if the RowVersion doesn't match the RowVersion on the database after calling SaveChanges().

Maybe I'm missing something here, but could you please briefly explain why does EF needs the updated concurrency token?

[roji]

Maybe I'm missing something here, but could you please briefly explain why does EF needs the updated concurrency token?

One reason is that a user may need to send the data to some disconnected web/mobile cilent (after having invoked SaveChanges()), including an up-to-date concurrency token (so that if that client later wants to make changes, they can do so; if they have an out-of-date token, the change will fail).

In a similar way, it should be possible to call SaveChanges() twice within the same unit-of-work, i.e. change the entity once, and then change it again based on some external input or something. If you don't fetch the token back during the first SaveChanges(), you can no longer perform the 2nd one (this is conceptually the same as the case I described above, only without a disconnected scenario).

[Virmak]

Thanks! That makes sense I overlooked those cases.

One solution inspired from https://github.com/dotnet/efcore/issues/29811#issuecomment-1352894192 , would be to register a SaveChangesInterceptor inside SqliteDbContextOptionsBuilderExtensions.UseSqlite() to avoid affecting other DB providers. This approach checks the property IsConcurrency token and doesn't use an interface to avoid contaminating the domain models

public override ValueTask<InterceptionResult<int>> SavingChangesAsync(DbContextEventData eventData, InterceptionResult<int> result, CancellationToken cancellationToken = default)
{
    var propertiesWithConcurrencyTriggers = eventData.Context!.ChangeTracker
        .Entries()
        .Where(e => e.State == EntityState.Modified)
        .SelectMany(x => x.Properties.Where(p => p.Metadata.IsConcurrencyToken));
    foreach (var property in propertiesWithConcurrencyTriggers)
    {
        property.OriginalValue = (int)property.CurrentValue + 1;
    }

    return new ValueTask<InterceptionResult<int>>(result);
}

What do you think of this approach?

[roji]

Yeah, that's similar in spirit to the "offline lock" approach that @ajcvickers pointed to above (#2195), though the proper fix would be to have this happen as part of the update pipeline, rather than in an interceptor that needs to go over all entity types and all properties on each SaveChangesAsync (that's not super efficient).

I'm going to go ahead and close this, as I don't think we'd want to implement the trigger-based approach (because of the limitation discussed above), and the rest seems to be covered by #2195. But @Virmak we can continue the conversation here and reopen if necessary.