Repository navigation
Add support for @stream - #10438
Open
michaelstaib wants to merge 23 commits into
Open
Add support for @stream#10438michaelstaib wants to merge 23 commits into
@stream#10438michaelstaib wants to merge 23 commits into
Conversation
Add StreamPage primitives for streaming paging
* Add end cursor wire format to Cursor, CursorFormatter and CursorParser * Add CreateLastPageCursor and CreateRelativeLastPageCursors to Page<T> * Add cursor helpers for StreamPage<T> * Share last-page cursor arithmetic between Page<T> and StreamPage<T> * Rename StreamPage cursor tests to match naming convention * Document null semantics for StreamPage cursor extensions
* Dispose batch pump source and lifetime immediately for an empty key set * Throw via ThrowHelper when the batch pump receives duplicate keys
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Critical paging and cursor defects, plus unresolved stream resource-cleanup issues, block approval.
Review effort: Lite
Findings: 3
Open (5)
What changed in this PR
Adds @stream support with streaming pages, batch routing, cursor handling, and paging options.
Changes:
- Adds streaming buffers, pumps, projections, and lifecycle management.
- Adds end-cursor parsing, formatting, and pagination helpers.
- Expands test coverage for streaming, batching, cursors, and paging arguments.
Unresolved critical and moderate issues affect IncludeItems, cursor compatibility and handling, cursor parsing, and stream resource cleanup.
| File | Reviewed change |
|---|---|
src/GreenDonut/test/GreenDonut.Data.Tests/StreamPageCursorTests.cs |
Tests streaming cursor behavior. |
src/GreenDonut/test/GreenDonut.Data.Tests/StreamPageBasicsTests.cs |
Tests buffering and replay. |
src/GreenDonut/test/GreenDonut.Data.Tests/StreamBatchPumpTests.cs |
Tests keyed stream routing. |
src/GreenDonut/test/GreenDonut.Data.Tests/PagingArgumentsTests.cs |
Tests IncludeItems. |
src/GreenDonut/test/GreenDonut.Data.Tests/PageTests.cs |
Tests last-page cursors. |
src/GreenDonut/test/GreenDonut.Data.Tests/Cursors/CursorParserTests.cs |
Tests end-cursor parsing. |
src/GreenDonut/test/GreenDonut.Data.Tests/Cursors/CursorFormatterTests.cs |
Tests end-cursor formatting. |
src/GreenDonut/src/GreenDonut.Data/Extensions/StreamPageCursorExtensions.cs |
Adds stream boundary cursors. |
src/GreenDonut/src/GreenDonut.Data/Extensions/LastPageCursorMath.cs |
Shares last-page cursor arithmetic. |
src/GreenDonut/src/GreenDonut.Data/Extensions/GreenDonutStreamPageExtensions.cs |
Adds streaming pagination helpers. |
src/GreenDonut/src/GreenDonut.Data/Extensions/GreenDonutPageExtensions.cs |
Adds page last-page helpers. |
src/GreenDonut/src/GreenDonut.Data/Cursors/CursorParser.cs |
Parses end cursors. |
src/GreenDonut/src/GreenDonut.Data/Cursors/CursorFormatter.cs |
Formats end cursors. |
src/GreenDonut/src/GreenDonut.Data/Cursors/Cursor.cs |
Adds end-cursor metadata. |
src/GreenDonut/src/GreenDonut.Data.Primitives/ValueCursorStreamPage.cs |
Implements value-based stream pages. |
src/GreenDonut/src/GreenDonut.Data.Primitives/StreamPage.cs |
Defines the streaming page API. |
src/GreenDonut/src/GreenDonut.Data.Primitives/PagingArguments.cs |
Adds item inclusion control. |
src/GreenDonut/src/GreenDonut.Data.Primitives/Internal/ThrowHelper.cs |
Centralizes streaming exceptions. |
src/GreenDonut/src/GreenDonut.Data.Primitives/Internal/StreamRow.cs |
Defines streamed rows. |
src/GreenDonut/src/GreenDonut.Data.Primitives/Internal/StreamPagePump.cs |
Owns shared page sources. |
src/GreenDonut/src/GreenDonut.Data.Primitives/Internal/StreamPageDefinition.cs |
Defines page streaming metadata. |
src/GreenDonut/src/GreenDonut.Data.Primitives/Internal/StreamPageBuffer.cs |
Buffers and resolves page rows. |
src/GreenDonut/src/GreenDonut.Data.Primitives/Internal/StreamBatchRow.cs |
Defines keyed batch rows. |
src/GreenDonut/src/GreenDonut.Data.Primitives/Internal/StreamBatchPump.cs |
Demultiplexes keyed streams. |
src/GreenDonut/src/GreenDonut.Data.Primitives/Internal/IStreamPageSource.cs |
Abstracts stream page sources. |
src/GreenDonut/src/GreenDonut.Data.Primitives/Internal/ElementProjectingSource.cs |
Projects streamed elements. |
src/GreenDonut/src/GreenDonut.Data.Primitives/GreenDonut.Data.Primitives.csproj |
Grants test internals access. |
src/GreenDonut/src/GreenDonut.Data.Primitives/ElementCursorStreamPage.cs |
Implements element-based cursors. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+70
to
+74
| /// with <see cref="IncludeTotalCount"/> set to <c>true</c>, otherwise the paging methods | ||
| /// raise an error. Defaults to <c>true</c> for every way of creating the arguments, | ||
| /// including <c>default</c>. | ||
| /// </summary> | ||
| public bool IncludeItems { get => !_excludeItems; init => _excludeItems = !value; } |
| int? PageIndex = null, | ||
| int? TotalCount = null) | ||
| int? TotalCount = null, | ||
| bool IsEndCursor = false) |
Comment on lines
+63
to
+64
| ArrayPool<byte>.Shared.Return(buffer); | ||
| return new Cursor([], offset, null, totalCount, IsEndCursor: true); |
|
|
||
| if (keys.Count > 0) | ||
| { | ||
| return new StreamBatchPump<TKey, TElement>(source, keys, lifetime); |
Comment on lines
+106
to
+113
| if (maxCursors < 0) | ||
| { | ||
| throw new ArgumentOutOfRangeException( | ||
| nameof(maxCursors), | ||
| "Max cursors must be greater than or equal to 0."); | ||
| } | ||
|
|
||
| await page.DrainAsync(cancellationToken).ConfigureAwait(false); |
Add ToStreamPageAsync and execute end cursors in EF paging
Add StreamPage DataLoader extensions and hash IncludeItems in branch keys
* Add a recording reader interceptor for EF paging tests * Add a scripted async source for GreenDonut.Data unit tests * Release the stream pump and lifetime when the source faults mid-stream * Consolidate the StreamPage and stream batch pump unit suites * Record the stream fault so later pulls rethrow it instead of touching the disposed source * Extend the mid-stream fault tests to cover the sibling page and replay * Add a StreamPage snapshot helper for EF paging tests * Add ToStreamPageAsync twins for the PagingHelperTests suite * Add a ToStreamPageAsync twin for the PagingTotalCountTests suite * Add a ToStreamPageAsync twin for the PagingInheritanceTests suite * Add ToStreamPageAsync twins for the PagingHelperIntegrationTests suite * Add streaming concurrency tests for a paused DbContext * Add AAA section markers to the total count stream tests * Drop the disputed HasPreviousPage flag from the after+last stream snapshot * Assert the executed command list for count-only and rows-only streams * Add a stream lifetime case for a drain-before-count ordering * Add ToStreamPageAsync twins for the RelativeCursorTests suite * Lock the end cursor test matrix for both paging APIs * Add cross-API cursor round-trip tests * Lock stale-total and trimmed-front-row read timing for the end cursor matrix * Add ToStreamPageAsync twins for the PagingHelperPostgreSqlNullableTests suite * Add ToStreamPageAsync twins for the PagingHelperSqlServerNullableTests suite * Clear the Npgsql connection pools after each EF paging test
* Prime only the batch pump's first row and drop rows for abandoned pages * Complete a sibling page whose channel outlives source exhaustion * Release the batch pump's source and lifetime when the priming read throws * Make StreamPage<T>.Empty report TotalCount 0 * Make StreamPage first-entry cursor helpers async and parse end cursors strictly (#9)
…ariant (#10) StreamPage<T> no longer carries lifecycle internals: the source owns priming, pages are built only already primed, and EnumerateEntriesAsync is renamed to GetEntriesAsync. Every failure path releases the source and the lifetime without masking the original exception, and the batch pump throws when a key reappears after its page completed. End cursors and the date and time cursor key serializers format and parse culture-invariantly, and the touched files follow the 120-column, one-argument-per-line and contract-comment conventions.
) * Add ToBatchStreamPageAsync and harden the EF streaming paging paths * Wrap over-120 paging calls and name boolean arguments * Trim narrative comments to their contract sentences * Restore the batch key-set contract and trim narrative comments * Wrap remaining partially wrapped paging calls one argument per line * Correct the batch outer ordering comment * Split ExpressionHelpers nested types into their own files * Split StreamEndCursorTests out of EndCursorTests * Split ReaderEvent out of RecordingReaderInterceptor * Split ScriptedAsyncDisposable out of ScriptedAsyncSource * Correct the StreamEndCursorTests header after the file split * Renamed BatchStreamExpression to BatchStreamQuery * refinements
…Source (#14) * Fix the batch pump fault contract for staged rows, unrequested keys, and drains * Fault the batch pump when a drain fails * Drop IStreamPageSource and fold its contract into the abstract StreamPageSource base class * Return the rented cursor buffer on every parse exit path
…e-invariantly (#15) * Include total count, relative cursors and null ordering in DataLoader paging branch keys Append markers for IncludeTotalCount, EnableRelativeCursors and NullOrdering only when they differ from their defaults, so the default paging combination keeps its existing branch key. Format enum values and First/Last in the branch key with the invariant culture so negative values hash identically regardless of the current culture. * Fix negative NullOrdering sizing and remove enum hash allocations
* Add cross-API paging equivalence and DataLoader streaming tests * Add relative offset and backward relative scenarios to the paging equivalence suite * Add fetch-based branch state tests for the stream paging DataLoader extensions * Rename StreamPage test cases to name ToStreamPageAsync
…on and tests (#17) * Fix batch stream flag and index derivation for empty backward relative pages * Fix EF paging validation order and polish paging tests Move the batch overflow and size validation in ToBatchPageAsync ahead of the per-key count query, matching the streaming method's order. Document the ArgumentExceptions for first/last and relative offset overflow on both single-page methods. Correct a stale comment naming the wrong flag trigger. Add last-plus-negative-offset overflow tests for all four paging methods. Rework the ReferencesParameter guard test so it actually exercises the member-collection branch instead of exiting on the type check. Assert ParamName and that no command ran on the null-key tests. Assert item identity and single lifetime release on the DataLoader end-to-end test, and wrap a declaration that exceeded 120 columns. * Cache the compiled batch key-set evaluator per expression shape * Slot every non-literal constant in the batch key-set shape cache * Key the batch key-set cache on expression structure * Add regression tests for the structural key-set cache * Remove the batch key-set evaluator cache * Remove cache-specific batch key-set tests
#20) * Add StreamPageConnection runtime types over StreamPage * Expose StreamPageConnection async members as cancellable methods * Unify the page info runtime types behind one async PageInfo * Wrap an overlong constructor call in the connection page info tests * Update the PostgreSQL schema snapshot for the unified async page info * Read offset page info flags synchronously in the pure resolvers * Delete PageInfoType and infer the single PageInfo type from the abstract page info * Update the federation paging snapshots for the shared page info type * Add the interceptor that marks the page info types shareable * Update the Types snapshots for the shared page info type * Update the Execution snapshots for the shared page info type * Update the Types.Queries snapshots for the shared page info type * Update the Data snapshots for the shared page info type * Update the Entity Framework data snapshots for the shared page info type * Update the AspNetCore schema snapshots for the shared page info type * Update the cost analysis paging snapshots for the shared page info type * Update the caching snapshots for the shared page info type * Update the introspection client snapshot for the shared page info type * Update the analyzer integration schema snapshots for the shared page info type * Update the Fusion AspNetCore snapshots for the shared page info type * Pin the mixed classic, PageConnection, StreamPageConnection and custom connection page info schemas * Register the page info shareable interceptor in the SchemaBuilder defaults * Tighten the page info schema tests and cover non-empty classic page info * Remove the stale snapshot of the renamed custom connection page info test * [CostAnalysis] Skip the default resolver cost for page info types * Pin the cost-free page info type and regenerate the cursor pagination snapshots * Regenerate the AspNetCore schema snapshots without cost on page info * Regenerate the analyzer integration schema snapshot without cost on page info * Regenerate the PostgreSQL schema snapshot without cost on page info * Skip automatic data resolver defaults for page info fields through a field flag * Pin page info without cache control and with an unchanged cache policy, and regenerate the caching snapshots * Expose the relative cursor fields of the page info type only when relative cursors are enabled * Regenerate the Types snapshots without relative cursor fields * Regenerate the Execution snapshots without relative cursor fields * Regenerate the Types.Queries snapshots without relative cursor fields * Regenerate the Analyzers integration snapshots without relative cursor fields * Regenerate the introspection client snapshot without relative cursor fields * Opt the page info cache control test into relative cursors and regenerate the caching snapshots * Regenerate the cost analysis snapshots without relative cursor fields * Regenerate the Data snapshots without relative cursor fields * Regenerate the Entity Framework snapshots without relative cursor fields * Regenerate the AspNetCore schema snapshots without relative cursor fields * Regenerate the Apollo Federation snapshots without relative cursor fields * Opt the explicit page cursor test into relative cursors and regenerate the page connection snapshots * Regenerate the Fusion gateway snapshots without relative cursor fields * Key the relative cursor field presence on the abstract page info members instead of the type name * Opt the ApolloFederation PageConnection schema tests into relative cursors * Rename the NoDataDefaults field flag to NotDataResolver and document its rationale * Move the PageInfo.Names nested class to the bottom of the type * Remove the sync accessors and pure resolvers from the offset CollectionSegmentInfo and flag its fields NotDataResolver
* Make StreamPageConnection Nodes virtual like PageConnection * [Analyzers] Generate connection and edge types for StreamPageConnection * [Analyzers] Bind StreamPageConnection async methods as connection fields * [Analyzers] Resolve StreamPageConnection page info through the unified PageInfo type * [Analyzers] Add a snapshot proving one PageInfo type across connection backings * [Analyzers] Add the PageInfo sharing snapshot * [Analyzers] Update the integration schema snapshot for the unified PageInfo type * [Analyzers] Add an executed PageInfo proof across classic, PageConnection and StreamPageConnection fields * [Analyzers] Add a classic paging backing to the PageInfo sharing snapshot * [Analyzers] Prove page info fields carry no cost or cache control on generated and classic connections * [Analyzers] Opt the PageInfo proofs into relative cursors and restore the four-field default schema snapshot
This branch has not been deployed
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.
No description provided.