CSHARP-6133: Validate cursor argument in IAsyncCursor/IAsyncCursorSource Extensions terminal operators - #2097
Conversation
…rce Extensions terminal operators
There was a problem hiding this comment.
Pull request overview
This PR tightens argument validation for IAsyncCursor and IAsyncCursorSource extension “terminal operators” (and related ForEachAsync overloads), ensuring ArgumentNullException is thrown with consistent parameter names and that invalid inputs don’t trigger unnecessary work.
Changes:
- Added
Ensure.IsNotNull(...)checks toIAsyncCursorSourceExtensionsterminal operators andForEachAsyncoverloads so nullsource/processoris rejected early. - Added
Ensure.IsNotNull(...)checks toIAsyncCursorExtensionsterminal operators, and adjustedForEachAsyncto preserve async-dispose behavior even whenprocessoris null. - Added/expanded unit tests covering null argument behavior for cursor/cursor-source terminal operators and
ForEachAsyncoverload variants.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/MongoDB.Driver.Tests/Core/IAsyncCursorSourceExtensionsTests.cs | Adds coverage asserting null source/processor behavior and ensuring queries aren’t executed when processor is null. |
| tests/MongoDB.Driver.Tests/Core/IAsyncCursorExtensionsTests.cs | Adds coverage asserting null cursor behavior and verifies disposal behavior when processor is null. |
| src/MongoDB.Driver/Core/IAsyncCursorSource.cs | Introduces explicit null checks for cursor-source terminal operators and ForEachAsync overloads. |
| src/MongoDB.Driver/Core/IAsyncCursor.cs | Introduces explicit null checks for cursor terminal operators and refines ForEachAsync null-processor handling to preserve disposal semantics. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // exhausted the thing and don't need it anymore. | ||
| await using (source.ConfigureAwait(false)) | ||
| { | ||
| Ensure.IsNotNull(processor, nameof(processor)); |
There was a problem hiding this comment.
Please add explanation why this validation is done not at the very beginning of the method. I got it, because we want to make sure the source will get disposed, but it's better to have comment so future us do not "standardize" the code.
No description provided.