Skip to content

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

Merged
merged 5 commits into from
May 24, 2023

Conversation

eramongodb
Copy link
Contributor

@eramongodb eramongodb commented May 24, 2023

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 with mock_server_replies_opmsg or the new mock_rs_replies_opmsg function, that explicitly specifying the reply contents (e.g. cursorId, firstBatch contents, etc.) as was intended by invoking *_replies_to_find with is_command == true.

@eramongodb eramongodb requested a review from kevinAlbs May 24, 2023 16:33
@eramongodb eramongodb self-assigned this May 24, 2023
Copy link
Collaborator

@kevinAlbs kevinAlbs left a 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);
Copy link
Collaborator

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_*.

Copy link
Contributor Author

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',"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
" 'ns': 'db.collection',"
" 'ns': 'test.test',"

@eramongodb eramongodb merged commit 39804da into mongodb:master May 24, 2023
@eramongodb eramongodb deleted the cdriver-4617-wip branch May 24, 2023 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants