Allow classes which inherit from DefaultResourceRepository more control of SaveChangesAsync

[jeff-becker-projekt202]

Description

Feature Request: Add methods in DefaultResourceRepository so that derived classes have access to the DBContext and an opportunity to intercept calls to _context.SaveChangesAsync() in CreateAsync, UpdateAsync, UpdateRelationshipsAsync, DeleteAsync.

Use Case: I have a fairly complex order-entry model. Any time any part of the hierarchy is modified I need to load the aggregate root and the subtotals for the entire hierarchy. Simply inheriting DefaultResourceRepository and overriding the relevant methods presents the following problems:

The derived class doesnt have direct access to the DBContext
Putting my recalculation call before the call to the base class means the entity instance will not have its relationships loaded which is required in this case to perform the calculation
Putting my recalculation call after the call the the base class would result in multiple calls to _dbContext.SaveChangesAsync() and thus multiple database transactions.

This can easily be solved with a minimum of architectural change by simply exposing a couple of well placed protected virtual methods on DefaultRespository<TResource,TId>. This way the default EFCore abstraction is maintained except for classes which explicitly inherit and override those methods. This way the changes are isolated to the places in the code which are already dependent on EFCore and don't affect the larger api surface.

Implementing this without the proposed virtual methods involved copy-pasta of the entire DefaultRepository class as well as a large chunk of internal reflection code.

Use Case

I am working on a point-of-sale system where any time any part of the tab is modified, the entire tab needs its subtotal and tax recalculated.

...

Environment

• JsonApiDotNetCore Version:
• Other Relevant Package Versions:

[jeff-becker-projekt202]

A class implementing this might look something like:

public class TabRespository : DefaultResourceRepository<Tab, long>
{
    public TabRespository(ITargetedFields targetedFields, IDbContextResolver contextResolver, IResourceGraph resourceGraph, IGenericServiceFactory genericServiceFactory) : base(targetedFields, contextResolver, resourceGraph, genericServiceFactory)
    {
    }
    protected override async Task BeforeSaveUpdateAsync(Tab tab)
    {
        //This method loads all the related entities with a minimum of round trips
        await _context.LoadTabRelationships(tab);

       //This calculates the subtotal with discounts and surcharges across all
       //Line items, modifiers, surcharges and addons
        tab.UpdateSubtotalsAndTax();
    }
}

[maurei]

Would it be feasible to use ResourceHooks as will be introduced in v4? In such approach you would put such business logic in a BeforeUpdate or AfterUpdate hook.

[jeff-becker-projekt202]

Can I have some time to dig into how those are wired into the database transaction? The overarching goal here is "Dont spread updates of related financial information across multiple transactions"

[maurei]

Of course, there is no rush here.

across multiple transactions

When you're taling about transactions, are you referring to SQL transactions as supported by EF Core? Or do you mean more generally speaking one block of code with a single reference to DbContext?

[jeff-becker-projekt202]

Thanks for the prompt reply on this. Specifically, I'm concerned about the SQL transactions and how the per-context and out of process entity caches interact with that. Per the article you linked any call to SaveChanges outside of an explicit transaction implicitly begins and commits a transaction. In our case the transaction of the Level 2 (out-of-process) redis cache is also tied into the calls to save changes.

I'd be open to implementing an over-arching unit of work abstraction which could also be useful in situations where related data is stored in differing data stores.

[maurei]

I think working with implicit transactions as per calling SaveChanges is going to be rather restrictive to work with. But since DbContext is scoped per request, injecting it in the ResourceHooks should yield the same instance the one used in the repository. I think we can leverage if we were to turn to IDbContextTransaction. Perhaps worth looking into would the following approach:

public class TabDefinition : ResourceDefinition<Tab>
{
    private readonly AppDbContext _dbContext;
    private IDbContextTransaction _transaction; 
    public TabDefinition(IResourceGraph resourceGraph, AppDbContext dbContext) : base(resourceGraph)
    {
        _dbContext = dbContext;
    }

    public override void BeforeUpdate( ... )
    {
        _transaction = _dbContext.Database.BeginTransaction();
        ... // do some stuff
    }

    public override void AfterUpdate( ... ) 
    {
        _transaction.Commit();
        _transaction.Dispose();
    }
}

Then any calls in BeforeUpdate as well as any calls in the repository layer should be wrapped in the same transaction.

[jeff-becker-projekt202]

That would work reasonably well for me with the following request: Would you accept a PR that makes those calls async?

[maurei]

What exactly do you need to be async?

[jeff-becker-projekt202]

It would be helpful if the Before/After Update/Create/Delete methods were async as I'll be doing database calls in them.

[maurei]

Having async hooks would only enable the caller, which is JADNCs internal HookExecutor, to await the execution of the hook, but there is no use-case for that because the internal flow depends on the outcome of the hooks so the hooks will always be run synchronously. This is needed for example when entities are removed from a API response or authorization is performed: having the API response finish before completion of authorization would be dangerous.

In terms of syntax it would be best to just use the synchronous versions of EF cores databases operations.