[CDRIVER-1954] Implement getters for internal fields of mongoc_find_and_modify_opts_t Created: 13/Dec/16  Updated: 05/Jan/17  Resolved: 05/Jan/17

Status: Closed
Project: C Driver
Component/s: libmongoc
Affects Version/s: None
Fix Version/s: 1.6.0

Type: New Feature Priority: Major - P3
Reporter: Samuel Rossi (Inactive) Assignee: A. Jesse Jiryu Davis
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

To test that the options are getting passed through correctly to the C driver, the C++ driver implements tests that "interpose" between the C driver's database calls and assert that the options are what they're supposed to be. In order to implement CXX-771 (https://jira.mongodb.org/browse/CXX-771), we need access to the `max_time_ms` field of `mongoc_find_and_modify_opts_t`, which currently is not public, and access to the other fields would help us more fully test the functionality of the `find_one_and_modify` calls. The implementation of this could either be making "getter" functions for the fields or just making the struct's implementation public rather than defined in a private header.



 Comments   
Comment by Githook User [ 05/Jan/17 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1954 getters for findandmodify opts struct
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/303bd2c7f77404d5e2b44aff301349ba3d229df1

Comment by J Rassi [ 16/Dec/16 ]

I agree with Sam that mongoc_find_and_modify_opts_t seems inconsistent with the other C driver options-like structs in that it cannot be introspected. Users of the C driver other than the C++ driver project could also find such introspection useful (for example: for being able to copy a mongoc_find_and_modify_opts_t, for debugging, or for their own unit testing).

Note that our specific need could also be satisfied with some sort of BSON serialization function (like mongoc_write_concern_append()) or equality function, as the particular use that Sam lays out in the ticket description is fundamentally for being able to tell if two mongoc_find_and_modify_opts_t structs are equal.

That said, this request is not a requirement for the C++ driver; if you decide to close this "Won't Fix", we'll simply test our find_and_modify() functionality with integration testing alone. The C++ driver is moving towards integration testing instead of unit testing for API functionality anyway.

Comment by A. Jesse Jiryu Davis [ 15/Dec/16 ]

We're willing to implement these getters if it's absolutely necessary. But it's a bummer to add more functions that we must maintain forever, only for the sake of C++ driver tests. Here's what I propose.

Follow the lead of the PyMongo maxTimeMS test. First, in your test suite, check if the server version is 2.6 or later, and that it has failpoints enabled. Here's how to check in the shell that failpoints are enabled:

> db.adminCommand({getCmdLineOpts:1})['parsed']['setParameter']

If you started the server with --setParameter enableTestCommands=1 then the output will be (in MongoDB 2.6+):

{ "enableTestCommands" : "1" }

If the server is older than 2.6 or doesn't have failpoints enabled, skip your maxTimeMS test.

Otherwise, configure maxTimeMS to always time out. In the shell:

> db.adminCommand({configureFailPoint: 'maxTimeAlwaysTimeOut', mode: 'alwaysOn'})
{ "ok" : 1 }

Now you can verify you're passing maxTimeMS to the server correctly, by asserting that a findandmodify with no maxTimeMS succeeds, but with maxTimeMS it fails with error code 50:

> db.runCommand({findandmodify: 'collection', update: {$set: {x: 1}}})
{ "value" : null, "ok" : 1 }
> db.runCommand({findandmodify: 'collection', update: {$set: {x: 1}}, maxTimeMS: 1})
{ "ok" : 0, "errmsg" : "operation exceeded time limit", "code" : 50 }

Now fix the server:

> db.adminCommand({configureFailPoint: 'maxTimeAlwaysTimeOut', mode: 'off'})
{ "ok" : 1 }

This is a little complicated, but it's how other drivers have tested their maxTimeMS implementation. It's a good end-to-end test. We implemented this failpoint, and we enable failpoints on Evergreen, precisely so that tests like this are possible.

Comment by A. Jesse Jiryu Davis [ 14/Dec/16 ]

We didn't deprecate mongoc_collection_find_and_modify_with_opts. It's still special: if you set a write concern, FAM_with_opts uses the write concern if the wire version >= 4. Whereas mongoc_client_write_command_with_opts and co. use the write concern if wire version >= 5.

(Because the server implemented write concern for findAndModify in MongoDB 3.2, and for other commands-that-write like dropCollection in MongoDB 3.4.)

Comment by Hannes Magnusson [ 14/Dec/16 ]

Didn't we more or less deprecate the mongoc_find_and_modify_opts_* with the introduction of mongoc_*_*_command_with_opts() ?
In which case, I think it'd be better idea to use those functions rather then adding more getters

Comment by A. Jesse Jiryu Davis [ 14/Dec/16 ]

You want these functions?:

mongoc_find_and_modify_opts_get_sort
mongoc_find_and_modify_opts_get_update
mongoc_find_and_modify_opts_get_fields
mongoc_find_and_modify_opts_get_flags
mongoc_find_and_modify_opts_get_bypass_document_validation
mongoc_find_and_modify_opts_get_max_time_ms
mongoc_find_and_modify_opts_extra

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