DRIVERS-1408 - Add guidance on adding _id fields to documents to CRUD spec

[NoahStapp]

Python implementation: mongodb/mongo-python-driver#1976

The new prose test only verifies that drivers correctly implement prepending _id for insert_one. Is it worthwhile to verify every other insert operation (insert_many, bulk_write, client_bulk_write) as well?

[jmikola]

Noted that this is already discussed in the client bulk write spec.

[NoahStapp]

Having this behavior discussed here for general CRUD operations makes sense to me, as the client bulk write spec follows from this broader context.

[jmikola]

The client bulk write spec stands on its own, except for a reference back to CRUD for modeling unacknowledged write results.

With my last comment, I just meant to acknowledge that no changes were needed to the client bulk write spec since it already addressed _id ordering. This was in the vein of previous PRs like #1644 that required updates to both specs.

[jmikola]

Perform an insert command

This is ambiguous, as the prose test could be implementing using the generic command runner, in which case I'd expect no ID generation. I'd suggest changing this to explicitly suggesting running an insertOne operation. You can extend that with insertMany and a bulkWrite if you want full test coverage.

[jmikola]

Is this only necessary in languages where the ordering is still not deterministic in the CommandStartedEvent's command document?

[NoahStapp]

This is the preferred way to verify the ordering of the document, as drivers may modify the document between emitting the CommandStartedEvent and the actual wire transfer of the command. For example, the Python driver re-orders _id to be the first field during BSON conversion, which takes place after the CommandStartedEvent would be emitted.

Verifying the order of the actual transmitted payload document ensures that the server receives exactly what we expect it to.

[nbbeeken]

I think it could potentially be rephrased if wire protocol parsing is difficult for drivers to achieve, but I agree it would be easy to assert _id is the first key in command monitoring and that not actually be the case when the wire message is produced.

I think perhaps for this test; we can encourage drivers to just use command monitoring as the "main path" set of assertions to implement. But we should encourage drivers to check that their wire message key order either matches their command events or that reordering is done when the wire message is built.

So for Node, I would write a test that asserts the JS object that is inspectable on the event (for any command) matches key order in BSON. Python may check that their wire messages are ordered correctly.

[NoahStapp]

If drivers are able to check wire messages directly, they should take that path. Otherwise, they should fall back to using command monitoring, ideally verifying that they don't modify field ordering before sending data over the wire.

[nbbeeken]

I agree, after all the point here is that the bytes are in an order that the server benefits from, if your command monitoring is a source of truth for such a thing you could/should use it but the real goal is in the wire message.

[nbbeeken]

I think it could potentially be rephrased if wire protocol parsing is difficult for drivers to achieve, but I agree it would be easy to assert _id is the first key in command monitoring and that not actually be the case when the wire message is produced.

I think perhaps for this test; we can encourage drivers to just use command monitoring as the "main path" set of assertions to implement. But we should encourage drivers to check that their wire message key order either matches their command events or that reordering is done when the wire message is built.

So for Node, I would write a test that asserts the JS object that is inspectable on the event (for any command) matches key order in BSON. Python may check that their wire messages are ordered correctly.

[jmikola]

LGTM w/ or w/o my suggestion.