-
Notifications
You must be signed in to change notification settings - Fork 455
Replace use of mock server *_replies_to_find when replying to OP_MSG requests #1280
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with minor comments.
void | ||
mock_rs_replies_opmsg (request_t *request, uint32_t flags, const bson_t *doc) | ||
{ | ||
mock_server_replies_opmsg (request, flags, doc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mock_rs_replies_*
functions do not depend on mock_rs_t
and forward arguments to the correspondingmock_server_replies_*
functions.
I suggest removing mock_rs_replies_*
functions and replacing calls with mock_server_replies_*
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed reference to mock_(server|rs)_*
in reply functions. Renamed to use a common reply_to_*
naming pattern instead.
tmp_bson ("{'ok': 1," | ||
" 'cursor': {" | ||
" 'id': {'$numberLong': '0'}," | ||
" 'ns': 'db.collection'," |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
" 'ns': 'db.collection'," | |
" 'ns': 'test.test'," |
Motivated by ongoing work on CDRIVER-4617 and CDRIVER-4630. Verified by this patch.
Improved validation and correctness checks for RPC message parsing in CDRIVER-4617 and CDRIVER-4630 (WIP) exposed several mock server tests that use
mock_(server|rs)_replies_to_find
to respond to OP_MSG requests.mock_(server|rs)_replies_to_find
assumes the request is an OP_QUERY and unconditionally accesses OP_QUERY flags in the request even if it is actually an OP_MSG.This PR replaces calls to
mock_(server|rs)_replies_to_find
that follow an OP_MSG request withmock_server_replies_opmsg
or the newmock_rs_replies_opmsg
function, that explicitly specifying the reply contents (e.g.cursorId
,firstBatch
contents, etc.) as was intended by invoking*_replies_to_find
withis_command == true
.