Skip to content

Integrate EF Core into ingestion unit of work#5615

Merged
johnsimons merged 1 commit into
masterfrom
john/ingest_part1
Jul 17, 2026
Merged

Integrate EF Core into ingestion unit of work#5615
johnsimons merged 1 commit into
masterfrom
john/ingest_part1

Conversation

@johnsimons

Copy link
Copy Markdown
Member

The IIngestionUnitOfWork interface is updated to IAsyncDisposable to properly manage asynchronous resources like ServiceControlDbContext and AsyncServiceScope.

EFIngestionUnitOfWork now manages a dedicated ServiceControlDbContext instance within its own AsyncServiceScope, persisting changes via SaveChangesAsync. A new IBodyStoragePersistence interface is introduced for abstracting message body storage.

The `IIngestionUnitOfWork` interface is updated to `IAsyncDisposable` to properly manage asynchronous resources like `ServiceControlDbContext` and `AsyncServiceScope`.

`EFIngestionUnitOfWork` now manages a dedicated `ServiceControlDbContext` instance within its own `AsyncServiceScope`, persisting changes via `SaveChangesAsync`. A new `IBodyStoragePersistence` interface is introduced for abstracting message body storage.
@johnsimons johnsimons self-assigned this Jul 17, 2026
@johnsimons
johnsimons enabled auto-merge July 17, 2026 05:28
var scope = serviceProvider.CreateAsyncScope();
var dbContext = scope.ServiceProvider.GetRequiredService<ServiceControlDbContext>();
var settings = scope.ServiceProvider.GetRequiredService<EFPersisterSettings>();
var unitOfWork = new EFIngestionUnitOfWork(scope, dbContext, storagePersistence, settings);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you are passing the container reference into the unit of work it seems like you could just resolve everything inside it's own constructor

However I'd recommend instead passing it in as an (optional?) IDisposable since that's all it cares about. An added bonus here is that it also removes the depend on Microsoft.Extensions.DI and makes testing it not require a container.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, I'll do this in the next PR


using System.IO;

public class MessageBodyFileResult

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the 'required' modifier for the props?

public async ValueTask DisposeAsync()
{
// Nothing to dispose yet
await dbContext.DisposeAsync();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'scope.dispose' on the next line should already take care of dbcontext

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this comment is implemented then you don't actually know that anymore

@johnsimons
johnsimons merged commit a6c17c6 into master Jul 17, 2026
213 of 219 checks passed
@johnsimons
johnsimons deleted the john/ingest_part1 branch July 17, 2026 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants