Skip to content

Edit failed messages in EF - #5721

Merged
rbev merged 2 commits into
masterfrom
rhys/edit-failures
Aug 11, 2026
Merged

rbev merged 2 commits into
masterfrom
rhys/edit-failures

Conversation

@rbev

@rbev rbev commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

This pull request implements support for editing failed messages in the EFCore persistence layer, including both SQL Server and PostgreSQL backends. It introduces a new FailedMessageEditEntity to track edit operations, updates the EFCore data model and migrations, and provides a concrete implementation of the edit manager. The changes also include enabling and testing the edit-and-retry feature in the test suites for both backends.

EFCore Data Model and Migrations

  • Added the FailedMessageEditEntity class to represent edits of failed messages, and created corresponding migrations for both PostgreSQL (20260810031820_AddFailedMessageEdits.cs) and SQL Server (20260810031725_AddFailedMessageEdits.cs). This includes new tables, primary keys, and indexes. [1] [2] [3]
  • Updated the EFCore DbContext (ServiceControlDbContext) and model configurations to include FailedMessageEditEntity and its configuration. [1] [2] [3]
  • Updated the model snapshots for both PostgreSQL and SQL Server to reflect the new entity and table. [1] [2]

Edit Manager Implementation

  • Implemented EditFailedMessagesManager and its data store, providing methods to get and set edit IDs, resolve failed messages (with correct timestamp stamping for retention), and persist changes. [1] [2]

Testing and Feature Enablement

  • Enabled the edit-and-retry tests (EditHandlerAuditTests.cs, EditMessageTests.cs) for both PostgreSQL and SQL Server test projects, and added focused EFCore-only tests to validate correct timestamp handling and edit ID persistence. [1] [2] [3]
  • Fixed an assertion in EditMessageTests.cs to use the correct unique message ID for edit tracking.

@rbev rbev self-assigned this Aug 10, 2026

@johnsimons johnsimons left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I added a few comments, but there is nothing really wrong it.
The manager pattern is interesting! Which is not introduced by this PR.

{
if (!Guid.TryParse(failedMessageId, out var uniqueMessageId))
{
return null!;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This feels like we should make the interface nullable?

Comment on lines +70 to +73
if (failedMessage == null)
{
throw new InvalidOperationException("No failed message loaded");
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am not the greatest fan of this design, where methods need to be called in a specific order!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah I don't love it, this interface is small enough that it probably doesn't need to do this at all. It's really just used for some reads and a TryUpsert()

@rbev
rbev force-pushed the rhys/edit-failures branch from 852ef3c to 4176754 Compare August 11, 2026 01:04
@rbev
rbev enabled auto-merge August 11, 2026 01:05
@rbev
rbev disabled auto-merge August 11, 2026 01:42
@rbev
rbev enabled auto-merge August 11, 2026 01:45
@rbev
rbev merged commit 03149ee into master Aug 11, 2026
50 checks passed
@rbev
rbev deleted the rhys/edit-failures branch August 11, 2026 01:56
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.

2 participants