[CDRIVER-3758] Do not validate options in read/write agnostic command helpers Created: 16/Jul/20  Updated: 31/Mar/22

Status: Backlog
Project: C Driver
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Major - P3
Reporter: Kevin Albertson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

libmongoc has several generic command helpers that are read/write agnostic.

  • mongoc_client_command()
  • mongoc_client_command_simple()
  • mongoc_client_command_simple_with_server_id()
  • mongoc_client_command_with_opts()
  • mongoc_database_command()
  • mongoc_database_command_simple()
  • mongoc_database_command_with_opts()
  • mongoc_collection_command()
  • mongoc_collection_command_simple()
  • mongoc_collection_command_with_opts()

These helpers should not be inspecting or validating options supplied by the user.

See this PR conversation for context: https://github.com/mongodb/mongo-c-driver/pull/662#discussion_r454365303

For example, most go through _mongoc_client_command_with_opts will call mongoc_cmd_parts_append_read_write to apply command options. In doing so, the server's wire version is checked for read/write concern support. This directly contradicts the read/write concern spec.

Let's audit these helpers to and remove unnecessary validation.



 Comments   
Comment by Akadoc 23 [ 29/Jul/20 ]

Hi ok I'm sorry for that, thank you for your explanations

Comment by Jeremy Mikola [ 29/Jul/20 ]

pimi.gonz@gmail.com: Thanks for expressing an interest in contributing. This particular issue is in our backlog so it's not something we're able to budget time for at the moment. I would suggest starting with CDRIVER tickets that have a "neweng" or "new-eng" label (see: this search filter). Those tickets should be more approachable for a first time contributor and require little (if any) guidance. Also, please review the libmongoc contributing guidelines before starting on anything if you've not seen that document already.

Comment by Akadoc 23 [ 29/Jul/20 ]

Hey, @Kevin Albertson so I've started to work on it:

First of all, I realized two distincts functions that end up to be called by the related helpers, that may check or validate read/write concern.
the first one is the function "mongoc_client_command_with_opts" that you have mentionned. I've noticed that it calls mongoc_read_write_opts_parse that checks if readConcern and writeConcern are specified in the options but I don't know if those checks are intended to be remove accordingly to this ticket. Furthermore so it's still assumptions, but I don't think it is supposed to be removed, (because I've seen this check a numerous time) there are checks on read and write Concern during a transaction of document between server and client (at line: 1969). Another condition about write concern is checked (at line 2048). And there is a last one check on read concern (at line 2073).

The second one is less frequently called "mongoc_cursor_new_with_opts" (in mongoc-cursor.c), I've noticed there is a check (at line 288) about the read concern in case there is specified options, but I don't know if it is supposed to be removed. 

So could you please tell me if I have to remove these checks, and if am I on the right way ?

Thank you

Comment by Akadoc 23 [ 24/Jul/20 ]

Hey I'm a new contributor and I want to help on the mongo C Driver, I've read the attached link to get more context, it's a bit blurry for now but I think by reading this again and exploring code it would help me. May I be assigned on it ?

Generated at Wed Feb 07 21:18:56 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.