Removing historic token entries generates exception when using an executing strategy.

[mdekok]

Describe the bug

Removing historic token entries generates exception when using an executing strategy using SQL Server.

To reproduce

I'm using the Quartz plugin as well as EF Core on a SQL Server with the EnableRetryOnFailure strategy set on the DbContext (options.UseSqlServer(storageSettings.ConnectionString, sqlServerOptions => sqlServerOptions.EnableRetryOnFailure());)

Exceptions (if any)

On job execution: One or more errors occurred. (The configured execution strategy 'SqlServerRetryingExecutionStrategy' does not support user-initiated transactions. Use the execution strategy returned by 'DbContext.Database.CreateExecutionStrategy()' to execute all the operations in the transaction as a retriable unit.)

My analysis

Enabling a executing strategy prevents you from initiating your own transactions (https://docs.microsoft.com/en-us/ef/core/miscellaneous/connection-resiliency#execution-strategies-and-transactions).

In OpenIddictEntityFrameworkCoreTokenStore.PruneAsync transactions are initiated to remove batches of tokens. If I remove EnableRetryOnFailure, pruning seems to work just fine.

I'm looking for a solution for this. I would like to keep EnableRetryOnFailure.

For me, using a explicit transaction in OpenIddictEntityFrameworkCoreTokenStore.PruneAsync seems not needed. I expect historic tokens are never modified and are only deleted by PruneAsync. So my solution would be removing the explicit transaction, but I probably overlook something.

Same for OpenIddictEntityFrameworkCoreAuthorizationStore.

Thanks for developing this package.

[kevinchalet]

The reason is actually explained here: https://github.com/openiddict/openiddict-core/blob/11d3a2eb8d30c0307c1f9098b8bcdeeeb88fd2e9/src/OpenIddict.EntityFrameworkCore/Stores/OpenIddictEntityFrameworkCoreAuthorizationStore.cs#L708-L711

The original idea was that by using a transaction, we reduced the risks of seeing the removal of 1 000 tokens fail just because one token was updated concurrently. That said, with the additional filters introduced to limit the scope of the query, it's likely something we indeed no longer need.

[kevinchalet]

Note: we use the same approach when removing applications/authorizations: in this case, getting rid of the transaction is way more problematic as authorizations and tokens can still be possibly created while the application is being removed.

Looks like we'll need to think more about that.

[mdekok]

Hi, maybe the fact SaveChanges itself is transactional can be used. Concurrency conflicts generate (should be very exceptional I expect) a DbUpdateConcurrencyException, which can be resolved in exception handling. Like it's done here: https://docs.microsoft.com/en-us/ef/core/saving/concurrency#resolving-concurrency-conflicts.

This also prevents using a repeatable read transaction and so keeps the housekeeping (pruning) isolated.

[vdurante]

I am facing the same issue. Has anyone been able to find a workaround or a solution to the issue? I will disable retryOnFailure atm...

[IliasP91]

hello I am also having the same issue, any workarounds for this other than disabling the quartz cleanup task?

[Bloodleeviy]

try to create your own type like this:

 public class YourOwnTokenStore<TContext> :
        YourOwnTokenStore<
            OpenIddictEntityFrameworkCoreAuthorization,
            OpenIddictEntityFrameworkCoreApplication,
            OpenIddictEntityFrameworkCoreToken, TContext, string>
        where TContext : DbContext
    {
        public YourOwnTokenStore(
            IMemoryCache cache,
            TContext context,
            IOptionsMonitor<OpenIddictEntityFrameworkCoreOptions> options)
            : base(cache, context, options)
        {
        }
    }
public class
        YourOwnTokenStore<TToken, TApplication, TAuthorization, TContext, TKey> :
            OpenIddictEntityFrameworkCoreTokenStore<TToken, TApplication, TAuthorization, TContext, TKey>
        where TAuthorization : OpenIddictEntityFrameworkCoreAuthorization<TKey, TApplication, TToken>
        where TApplication : OpenIddictEntityFrameworkCoreApplication<TKey, TAuthorization, TToken>
        where TToken : OpenIddictEntityFrameworkCoreToken<TKey, TApplication, TAuthorization>
        where TContext : DbContext
        where TKey : IEquatable<TKey>
    {
        public YourOwnTokenStore(
            IMemoryCache cache,
            TContext context,
            IOptionsMonitor<OpenIddictEntityFrameworkCoreOptions> options) : base(cache, context, options)
        {
        }

then override PruneAsync like this:

  public override async ValueTask PruneAsync(DateTimeOffset threshold, CancellationToken cancellationToken)
        {
                ...
                var strategy = Context.Database.CreateExecutionStrategy();
                await strategy.ExecuteAsync(async () =>
                {
                    await using var transaction = await Context.Database.BeginTransactionAsync(IsolationLevel.RepeatableRead, cancellationToken);
                    Context.RemoveRange(tokens);

                    try
                    {
                        await Context.SaveChangesAsync(cancellationToken);
                        await transaction?.CommitAsync(cancellationToken)!;
                    }

                    catch (Exception exception)
                    {
                        exceptions ??= new List<Exception>(capacity: 1);
                        exceptions.Add(exception);
                    }
                });
              ...
}

then register like this:

Services.AddScoped<IOpenIddictTokenStore<OpenIddictEntityFrameworkCoreToken>,
            YourOwnTokenStore<YourDbContext>>();

[IliasP91]

finally got some spare time and looked into this, I managed to solve it by creating my own stores for Token and Authorization, inheriting the EFcore implementation (the one using a TKey as I do override it) and overriding PruneAsync method in both of them, I copied most of its implementation from the following links except the part that creates a db transaction, an I wrapped

Context.RemoveRange(authorizations);

try
{
    await Context.SaveChangesAsync(cancellationToken);
    await transaction.CommitAsync(cancellationToken);
}
catch (Exception exception) when (!IsFatal(exception))
{
    exceptions ??= new List<Exception>(capacity: 1);
    exceptions.Add(exception);
}

in

var strategy = Context.Database.CreateExecutionStrategy();

await strategy.ExecuteAsync(
    async () =>
    {
        // Note: new tokens may be attached after the authorizations were retrieved
        // from the database since the transaction level is deliberately limited to
        // repeatable read instead of serializable for performance reasons). In this
        // case, the operation will fail, which is considered an acceptable risk.
        await using var transaction = await Context.Database.BeginTransactionAsync(IsolationLevel.RepeatableRead, cancellationToken);

        Context.RemoveRange(authorizations);

        try
        {
            await Context.SaveChangesAsync(cancellationToken);
            await transaction.CommitAsync(cancellationToken);
        }
        catch (Exception exception) when (!IsFatal(exception))
        {
            exceptions ??= new List<Exception>(capacity: 1);
            exceptions.Add(exception);
        }
    });`

which handles transactions properly with EF retries

a small note is that I did not directly register my stores in the DI but I used an extension method of openiddict to do it like this:

// Register the OpenIddict core components.
.AddCore(
  options =>
  {
      // Configure OpenIddict to use the Entity Framework Core stores and models.
      // Note: call ReplaceDefaultEntities() to replace the default OpenIddict entities.
      options.UseEntityFrameworkCore()
             .UseDbContext<AppDbContext>()
             .ReplaceDefaultEntities<Guid>();

      // override stores
      options.AddTokenStore<YourOwnTokenStore<AppDbContext, Guid>>();
      options.AddAuthorizationStore<YourOwnAuthorizationStore<AppDbContext, Guid>>();

      // Enable Quartz.NET integration.
      options.UseQuartz();
  })

stores: https://github.com/openiddict/openiddict-core/blob/dev/src/OpenIddict.EntityFrameworkCore/Stores/OpenIddictEntityFrameworkCoreAuthorizationStore.cs https://github.com/openiddict/openiddict-core/blob/dev/src/OpenIddict.EntityFrameworkCore/Stores/OpenIddictEntityFrameworkCoreTokenStore.cs

@Bloodleeviy thank you for the pointer, much appreciated

[WolfspiritM]

Having the same issue right now. Any news except of creating a custom token store?

[IliasP91]

Having the same issue right now. Any news except of creating a custom token store?

nope the above has been working just fine so far