Repository navigation
Handle empty and duplicate keys in the StreamPage batch pump - #3
Merged
Merged
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Empty-key cleanup can leak the lifetime if source disposal throws.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
This PR hardens StreamBatchPump for empty and duplicate key sets.
Changes:
- Adds async factory creation with empty-key cleanup.
- Rejects duplicate keys via
ThrowHelper. - Updates tests for the new API and edge cases.
| File | Description |
|---|---|
src/GreenDonut/test/GreenDonut.Data.Tests/StreamBatchPumpTests.cs |
Updates construction and adds edge-case tests |
src/GreenDonut/src/GreenDonut.Data.Primitives/Internal/ThrowHelper.cs |
Adds duplicate-key exception creation |
src/GreenDonut/src/GreenDonut.Data.Primitives/Internal/StreamBatchPump.cs |
Implements factory creation, cleanup, and duplicate validation |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
michaelstaib
added this pull request to stack #7
September 27, 2026 10:23
michaelstaib
force-pushed
the
mst/primitives-pump-fixes
branch
from
September 27, 2026 20:58
d2e9ac0 to
f930173
Compare
michaelstaib
force-pushed
the
mst/primitives-pump-fixes
branch
from
September 27, 2026 21:40
f930173 to
5ed8e5d
Compare
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.
This tightens two edge cases in the batch pump from #1. An empty key set now releases the source and the lifetime right away instead of holding them forever. Duplicate keys now throw a clear
ArgumentExceptionthroughThrowHelper, and the message names the key. To make the empty case possible, the pump's public constructor became a staticCreateAsyncfactory that returns null when there is nothing to serve. Reviewers should note that a duplicate-key throw still leaves disposal to the caller; the EF batch method in a later PR is required to de-duplicate keys before calling the pump.Closed tickets: hc-fork-1-dno.4 (6b72e5b), hc-fork-1-dno.5 (d2e9ac0).
Stack: sits on #2 (end cursors and StreamPage cursor helpers,
mst/cursors-streampage).🤖 Generated with Claude Code