Remove redundant DeleteMessages(StoredId, positions) IMessageStore overload#174
Merged
Merged
Conversation
…erload Message positions are globally-unique identity values, so the StoredId filter was redundant. ExistingMessages.Remove now routes through the positions-only DeleteMessages overload, and the two-arg overload is removed from the interface, all four store implementations, and the PostgreSQL/MariaDB/SqlServer SQL generators. The shared message-store test templates are migrated to the positions-only overload (method names unchanged, so per-store override files are untouched), giving that overload cross-store coverage it previously lacked.
The test postponed to new DateTime(1_000_000) - a year-0001 timestamp that is already expired - so the PostponedWatchdog could resume the function and flip its status from Postponed to Executing before the assertions ran. Postpone to a future timestamp (DateTime.UtcNow.AddDays(1)) instead, matching the sibling PostponingExistingActionFromControlPanelSucceeds test, which closes the race.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Removes
Task DeleteMessages(StoredId storedId, IEnumerable<long> positions)fromIMessageStore. Message positions are globally-unique identity values, so theStoredIdfilter was redundant — the positions-only overloadDeleteMessages(IReadOnlyList<long> positions)covers the same need.ExistingMessages.Remove(the sole production caller) now routes through the positions-only overload.InMemoryFunctionStore,PostgreSqlMessageStore,MariaDbMessageStore,SqlServerMessageStore), and the now-orphanedSqlGenerator.DeleteMessages(StoredId, …)in the PostgreSQL/MariaDB/SqlServer generators.Tests
MessageClearer) and is now exercised across all stores.DeleteMessagesWithNonExistentPositionsDoesNotThrowwas reworked to delete a real, already-removed position rather than a hardcoded999/1000; without theStoredIdfilter an arbitrary number could collide with another flow's globally-unique position in a shared database.ControllableMessageStoretest double.Verification
DeleteMessages*+MessageClearer: 10/10DeleteMessages*: 5/5DeleteMessages*: 5/5DeleteMessages*: 5/5🤖 Generated with Claude Code
Also: flaky test fix (unrelated)
CI surfaced a pre-existing flaky test,
PostponingExistingFunctionFromControlPanelSucceeds, while this PR was in review. It postponed tonew DateTime(1_000_000)— a year-0001 timestamp that is already expired — so thePostponedWatchdogcould resume the function and flip its statusPostponed → Executingbefore the assertions ran. Fixed by postponing to a future timestamp (DateTime.UtcNow.AddDays(1)), matching the siblingPostponingExistingActionFromControlPanelSucceedstest. Not related to theDeleteMessageschange, but included here since it was blocking this PR's CI.