Add two methods for consuming repositories in scenarios where repositories could be longer lived (e.g. Blazor component Injections)#289
Conversation
…ories could be longer lived (e.g. Blazor component Injections) - BREAKING CHANGE - requires support for netstandard2.0 to be dropped from Ardalis.Specification.EntityFrameworkCore.csproj in order to make use of IDbContextFactory - Add IRepositoryFactory interface and EFRepositoryFactory concrete implementation to encapsulate the 'Unit of Work' principle at the repository level, consuming DbContextFactories from DI containers such as those added using the .AddDbContextFactory method, following blazor best practices for managing DbContext lifetimes - Add ContextFactoryRepositoryBaseOfT.cs abstract implementation of IRepositoryBase<T> which again consumes DbContextFactories from DI containers such as those added using the .AddDbContextFactory method but creates a new instance of the DbContext for every method call in the repository. This breaks Entity Framework change tracking so Update and Delete methods will have to be overloaded in concrete implementations using the TrackChanges method on the context.
| /// <inheritdoc/> | ||
| public async Task<TResult?> FirstOrDefaultAsync<TResult>(ISpecification<TEntity, TResult> specification, CancellationToken cancellationToken = default) | ||
| { | ||
| await using var dbContext = this.dbContextFactory.CreateDbContext(); |
There was a problem hiding this comment.
Since this is using using won't it make change tracking and later updates of the fetched entity impossible? The dbContext that originally is tracking the entity will be disposed by the end of this method, and so when a savechanges is called there will be an error saying "entity is already tracked by another dbcontext" or something equivalent, right? If not, can you write another few tests demonstrating that your solution works for fetch-change-save operations?
There was a problem hiding this comment.
Apologies Steve, I didn't give that fact much attention in my initial request.
Yes, I would expect such an error with this approach. I'll add a virtual protected method which invokes the TrackGraph method inside the using to solve this. The original intention was to allow for maximum flexibility but on reflection a default approach to managing the change tracker is definitely appropriate.
I just need to work through getting the error we're expecting to present itself in tests to ensure it's being handled appropriately.
I'll post another update here when the above is completed.
There was a problem hiding this comment.
I'll add a virtual protected method which invokes the TrackGraph method inside the using to solve this.
@jasonsummers, was this done? If so, where? Either way, can you explain this for me please?
Thanks,
Matt
There was a problem hiding this comment.
@mwasson74, this wasn't completed in the end, mainly because the only solutions I could come up with added too much opinion into the library.
Essentially, due to the prolonged lifecycle of Blazor (and WPF/UWP/MAUI etc) apps, the new ContextFactoryRepositoryBaseOfT class instantiates a new instance of the DBContext every time a method is invoked. This means that all of the Entity Framework Change Tracking goodness is lost.
From a DDD perspective, this actually makes sense, whether or not an Entity has changed is a subject for the Entity to manage itself and not be reliant on a 3rd party to deduce.
What this means is that you need to implement change tracking manually within your solution. I think the only method you'll have to overload is the UpdateAsync method since that's the only one which really needs to know what's changed.
Sorry this is so vague, it's been a while since I've looked at this. Reply back here if you need more info. I'll try and carve out some time to write a proper doc for this as well.
|
Let me know when you want me to review again @jasonsummers . Are you thinking it's good to go now? |
…sts for EFRepositoryFactory
46dc370 to
79617dd
Compare
|
Hi @ardalis, I think this is good to go. Unit tests are have been added to cover all of the permutations I could think of which would cause the |
|
Hi, We've been holding back this PR for a while. It has to do with the breaking change, we have to drop the support for
@ardalis what are your thoughts here? Should we drop everything prior EF Core 6? |
|
I think we should support Blazor Server (and its wonky lifetime requirements). If that means we need to drop forward compatibility with netstandard 2.0 I think that's probably fine. We can document which version is the latest one to support that TFM in the README. What do you think about our version relative to .NET's version, @fiseni ? On the one hand I kind of like(d) the idea of tracking with .NET, so when .NET 8 ships we'd aim to have a Specification 8.0 to go with it. But on the other hand we currently have folks who are confused about which version of EF Core our library supports because of the version number differences. Maybe it would be better if our version number were, say, 17 or something so it was clear that it wasn't meant to be anywhere near .NET's or EF's version. I mention the version number only because this breaking change would require us to increment our majorversion, and if we do it soon-ish, it will obviously be well before .NET 8 ships in Nov 2023. |
|
Hi @fiseni. Wondering, can you provide a general time line for releasing these changes? Thanks so much! |
|
Yes, in the next two weeks, around 15th May. |
|
Hi @jasonsummers, nice PR. Just what I've been looking for to use in my Blazor Server app. How is it supposed to be dependency-injected via Program.cs? Currently, I use it like this, which works. Unfortunately, that is only for my concrete I also tried with the following two lines of code. And additionally, tried to create my own kind of wrapper for EFRepository, which didn't work out, unfortunately. I have the feeling that I kind of miss something, but unfortunately didn't find any documentation or further explanations about My goal is to register Do you have any hint or advice for me? Thanks in advance! :-) |
|
Hi @pbru87. Thanks for the feedback. Apologies in advance, my explanation is going to be a little rusty because this PR was merged 3 years ago and in truth I've not had the need to use it recently. This PR did not implement the functionality to register empty generics. It surprises me that the following lines work because the Types are (or at least were) required in order for the strong-typing of the specifications to work correctly: Unfortunately you're going to have to register repositories for each type separately, based on my admittedly very faded memory of this. |
|
Hi @jasonsummers. Thanks for your reply! That information already helps a lot (to know I didn't missed something or that I'd make it complicated out of no reason). Yeah, for the regular repositories this works just fine. I use it – together with 'Specifications' – in production in an classic 'Razor Pages' application. (btw: It's also used in the eShopOnWeb sample application.) – I also started using this in my Blazor Server app, but then realized it doesn't fully fit with the Blazor logic. So, I later stumbled upon this PR and your 'Factory' approach. 🙂 |

Provides two solutions for #280