[CXX-1152] Context enforcement for BSON stream builder doesn't play nicely with statements that leave open sub-documents or sub-arrays Created: 01/Dec/16  Updated: 20/Apr/17  Resolved: 20/Apr/17

Status: Closed
Project: C++ Driver
Component/s: BSON, Documentation
Affects Version/s: None
Fix Version/s: 3.2.0-rc0

Type: Bug Priority: Major - P3
Reporter: J Rassi Assignee: Samuel Rossi (Inactive)
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
depends on CXX-1250 mongocxx implementation should use bs... Closed
Related
is related to CXX-1202 Document how to build array in a loop... Closed

 Description   

BSON stream builders feature the ability to stream keys and values across multiple statements, including values that leave open new sub-documents and sub-arrays. As a result, the type system cannot have knowledge of what the current level of depth is for a BSON stream builder, or what the type of its current context is. This leads to unintuitive behaviors.

Consider the following code snippet (modified from the test case "[] with large nesting levels") that uses a document stream builder to build the document {"a": {"b": {"c": 1}}}:

// Example 1:
bsoncxx::stream::builder::document doc;
doc << "a" << open_document;
doc << "b" << open_document;
doc << "c" << 1;
doc << close_document;
doc << close_document;

This above code snippet correctly builds this document. However, this code compiles only because all of the above statements happen to be valid in key context (since the type of "doc" is a derived class of bsoncxx::builder::stream::key_context).

Things start to get wonky when attempting to use stream builders in a statement where multiple levels of depth are backtracked:

// Example 2:
bsoncxx::stream::builder::document doc;
doc << "a" << open_document << "b" << open_document;
doc << "c" << 1;
doc << close_document << close_document;  // Doesn't compile!  Can't append close_document to a closed_context.

Or, when a statement leaves open a sub-document or sub-array which provides a different context than the top-level BSON datum type provides:

// Example 3:
bsoncxx::stream::builder::document doc;
doc << "x" << open_array;
doc << 1;  // Doesn't compile!  "doc" isn't an array_context.
doc << close_array;  // Doesn't compile!  "doc" isn't an array_context.

The fact that "example 1" compiles and runs but "example 2" and "example 3" fail to compile reveals a fundamental design flaw in the stream builder. The stream builder should be changed such that either all three examples compile, or all three examples fail to compile.



 Comments   
Comment by Githook User [ 20/Apr/17 ]

Author:

{u'username': u'saghm', u'name': u'Saghm Rossi', u'email': u'saghmrossi@gmail.com'}

Message: CXX-1152 Context enforcement for BSON stream builder doesn't play nicely with statements that leave open sub-documents or sub-arrays
Branch: master
https://github.com/mongodb/mongo-cxx-driver/commit/21ebdc1856bbe12136c9755aecb06cd2ffd3ceaf

Comment by David Golden [ 14/Apr/17 ]

Let's fix this via documentation that explains it and warns users not to do it, rather than trying to fix it in the code. We're generally discouraging the stream builder approach for general use anyway.

Comment by Andrew Morrow (Inactive) [ 03/Dec/16 ]

I would consider both examples where the intermediate result type was not captured to be ill formed - the fact that they work is accidental. How about using _attribute_((warn_unused_result)) (via an appropriate compiler macro) on the functions that return context that is intended to be captured?

Comment by J Rassi [ 01/Dec/16 ]

My current ideas of possible solutions for this issue are as follows (all three examples compile with solution #1, whereas all three examples fail to compile with solutions #2-#5):

  1. Merge key_context, value_context, array_context, close_context, and single_context into a single class, to leave the type system out of the business of enforcing that a streaming statement is well-formed. The builders already use exceptions to communicate misusage (for example, the expression "builder::stream::document{} << builder::stream::close_document" already throws an exception with code k_no_document_to_close), so statements that don't compile today could simply be converted to statements that throw a "wrong context" error of some sort.
  2. Remove the open_document_type and open_array_type types, such that a single statement can never leave a sub-document or sub-array open (and thus change the builder's logical context). Users can build sub-documents or sub-arrays using single context lambdas.
  3. Use some sort of visibility or template magic to make any streaming statement not compile if it leaves open a sub-document or sub-array.
  4. Make the bsoncxx::builder::stream::document and bsoncxx::builder::stream::array types not derived classes of key_context or array_context, but instead types that support one-time conversions to key_context or array_context. The only way users would be able to append to sub-documents or sub-arrays across multiple statements would be to save the current context, as follows:

    builder::stream::document doc;
    auto doc_key_context = builder::stream::key_context{doc};
    auto subdoc_key_context = doc << open_document;
    subdoc_key_context << "a" << 1 << close_document; 
    

  5. Temporary hack solution: make builder::stream::document and builder::stream::array coercible to any context. For example:

    builder::stream::document doc;
    doc << "x" << open_array;
    builder::stream::array_context{doc} << 1;
    builder::stream::array_context{doc} << close_array;
    

Generated at Wed Feb 07 22:01:34 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.