RUBY-3213 Add Database#cursor_command to create a cursor from a command response#3055
RUBY-3213 Add Database#cursor_command to create a cursor from a command response#3055comandeo-mongo wants to merge 1 commit into
Conversation
…nd response Implements the runCursorCommand API from the run-command spec. The command is run unmodified and its response is parsed as a cursor; if the response has no cursor field an Error::InvalidCursorOperation is raised. The implicit session is reused across getMores and ended when the cursor is exhausted or closed. getMore commands can be configured with batch_size, max_time_ms and comment options. The command is never retried. Adds the runCursorCommand unified test fixtures and the runCursorCommand / createCommandCursor operations to the unified test runner.
There was a problem hiding this comment.
Pull request overview
This PR adds support for the run-command spec’s runCursorCommand API by introducing a new public Mongo::Database#cursor_command method that executes an arbitrary cursor-returning command and materializes the response into a Mongo::Cursor, including configurable getMore options.
Changes:
- Added
Mongo::Database#cursor_command, a minimalCursorCommandView, and a newOperation::CursorCommand(+ result parsing) to build cursors from command responses. - Updated
Mongo::Cursor#get_more_operationto optionally send a getMore-specificmaxTimeMSwhen the backing view supports it (leaving existing find/aggregate cursors unchanged). - Added unified spec fixtures and runner support operations for
runCursorCommand/createCommandCursor, plus a unified DDL runner tweak to support entity saving for a capped collection fixture.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/spec_tests/data/run_command_unified/runCursorCommand.yml | Adds unified spec fixtures covering cursor-command behavior (sessions, LB pinning, getMore options, tailable). |
| spec/runners/unified/support_operations.rb | Implements unified runner operations to execute runCursorCommand and create/save command cursors as entities. |
| spec/runners/unified/ddl_operations.rb | Updates createCollection unified operation to honor saveResultAsEntity. |
| lib/mongo/operation/cursor_command/result.rb | Adds a result wrapper that extracts cursor subdocument fields for cursor construction. |
| lib/mongo/operation/cursor_command/op_msg.rb | Defines OP_MSG command execution for cursor commands, sending selector verbatim. |
| lib/mongo/operation/cursor_command.rb | Introduces the CursorCommand operation type. |
| lib/mongo/operation.rb | Requires the new cursor command operation. |
| lib/mongo/database/cursor_command_view.rb | Adds a minimal view object used by cursors created from command responses. |
| lib/mongo/database.rb | Adds Database#cursor_command implementation and view-option extraction helper. |
| lib/mongo/cursor.rb | Allows getMore to include a getMore-specific maxTimeMS when exposed by the view. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| server = selector.select_server(cluster, nil, session) | ||
| result = if server.load_balancer? | ||
| # The connection is checked in by the cursor when it is drained. | ||
| connection = server.pool.check_out(context: context) | ||
| op.execute_with_connection(connection, context: context, options: execution_opts) | ||
| else | ||
| op.execute(server, context: context, options: execution_opts) | ||
| end |
There was a problem hiding this comment.
I'm unfamiliar with the spec, here, but this does seem like a legitimate omission. Should this be wrapped in with_overload_retry? Collection::View::Iterable, for example, uses with_overload_retry when sending the initial cursor command.
| unless cursor | ||
| connection.connection_pool.check_in(connection) if connection && !connection.pinned? | ||
| session.end_session if session && session.implicit? | ||
| end |
There was a problem hiding this comment.
I'm pretty sensitive to the risk of leaking connections (I think I'm kind of traumatized by that help ticket I worked on recently...!) so comment definitely catches my attention. I'm not entirely sure that Copilot is proposing the right solution here, though -- assuming something failed during Cursor.new, would the connection have been pinned?
| runOnRequirements: | ||
| - serverless: forbid | ||
| operations: |
jamis
left a comment
jamis
left a comment
There was a problem hiding this comment.
Approving this, but wanted to flag two of the issues that Copilot raised. I'm not familiar enough with the spec, here, to know if they actually have merit.
| server = selector.select_server(cluster, nil, session) | ||
| result = if server.load_balancer? | ||
| # The connection is checked in by the cursor when it is drained. | ||
| connection = server.pool.check_out(context: context) | ||
| op.execute_with_connection(connection, context: context, options: execution_opts) | ||
| else | ||
| op.execute(server, context: context, options: execution_opts) | ||
| end |
There was a problem hiding this comment.
I'm unfamiliar with the spec, here, but this does seem like a legitimate omission. Should this be wrapped in with_overload_retry? Collection::View::Iterable, for example, uses with_overload_retry when sending the initial cursor command.
| unless cursor | ||
| connection.connection_pool.check_in(connection) if connection && !connection.pinned? | ||
| session.end_session if session && session.implicit? | ||
| end |
There was a problem hiding this comment.
I'm pretty sensitive to the risk of leaking connections (I think I'm kind of traumatized by that help ticket I worked on recently...!) so comment definitely catches my attention. I'm not entirely sure that Copilot is proposing the right solution here, though -- assuming something failed during Cursor.new, would the connection have been pinned?
Implements the
runCursorCommandAPI from the run-command spec asMongo::Database#cursor_command.The method runs a cursor-returning command and parses the response into a
Mongo::Cursor. The user's command is sent unmodified. If the response does not contain acursorfield,Mongo::Error::InvalidCursorOperationis raised. The implicit session is created if none is provided, reused across allgetMores, and ended when the cursor is exhausted or closed.getMorecommands can be configured withbatch_size,max_time_ms, andcomment. The command is never retried, per the spec.Changes
Implementation:
lib/mongo/database.rb— new public#cursor_commandand a private options helper.lib/mongo/database/cursor_command_view.rb— minimal view theCursorreads from when built from a command response (carries getMore options, cursor type, timeout mode).lib/mongo/operation/cursor_command.rb(+op_msg.rb,result.rb) — the operation and a result that parses the cursor subdocument (firstBatch,ns,id).lib/mongo/operation.rb— require the new operation.lib/mongo/cursor.rb—getMorenow sends a getMore-specificmaxTimeMS, routed via a uniquely named view method guarded byrespond_to?so find/aggregate cursors are unaffected.Tests:
spec/spec_tests/data/run_command_unified/runCursorCommand.yml— unified fixtures from the spec.spec/runners/unified/support_operations.rb—runCursorCommandandcreateCommandCursoroperations.spec/runners/unified/ddl_operations.rb—createCollectionnow honorssaveResultAsEntity(needed by the tailable fixture).Test plan
Run against a local replica set:
bundle exec rspec spec/spec_tests/run_command_unified_spec.rb— 21 examples, 0 failures, 4 pending. The pending cases require sharded (checkMetadataConsistency) and load-balanced topologies, which the local replica set cannot provide; they run in Evergreen.bundle exec rspec spec/mongo/cursor_spec.rb— 42 examples, 0 failures (no regression from theget_more_operationchange).bundle exec rspec spec/mongo/database_spec.rb— 112 examples, 0 failures.bundle exec rspec spec/integration/cursor_reaping_spec.rb spec/integration/get_more_spec.rb spec/mongo/collection/view_spec.rb— 0 failures.bundle exec rubocopon all changed files — no offenses.Jira: https://jira.mongodb.org/browse/RUBY-3213