CXX-1116 result::insert_many needs to hold on to owned _id elements

Fixes a bug where the the return value of
result::insert_many::inserted_ids() could contain dangling references.

Author: jrassi

src/mongocxx/insert_many_builder.cpp

@@ -39,25 +39,30 @@ options::bulk_write make_bulk_write_options(const options::insert& insert_option
 }  // namespace
 
 insert_many_builder::insert_many_builder(const options::insert& options)
-    : _writes{make_bulk_write_options(options)}, _inserted_ids{}, _index{0} {};
+    : _writes{make_bulk_write_options(options)}, _inserted_ids{} {
+}
 
 void insert_many_builder::operator()(const bsoncxx::document::view& doc) {
+    bsoncxx::builder::stream::document id_doc;
     if (!doc["_id"]) {
-        bsoncxx::builder::stream::document new_document;
-        new_document << "_id" << bsoncxx::oid() << bsoncxx::builder::stream::concatenate(doc);
+        id_doc << "_id" << bsoncxx::oid{};
 
+        bsoncxx::builder::stream::document new_document;
+        new_document << bsoncxx::builder::stream::concatenate(id_doc.view())
+                     << bsoncxx::builder::stream::concatenate(doc);
         _writes.append(model::insert_one{new_document.view()});
-        _inserted_ids.emplace(_index++, new_document.view()["_id"]);
     } else {
+        id_doc << "_id" << doc["_id"].get_value();
+
         _writes.append(model::insert_one{doc});
-        _inserted_ids.emplace(_index++, doc["_id"]);
     }
+    _inserted_ids.append(id_doc.view());
 };
 
 stdx::optional<result::insert_many> insert_many_builder::insert(collection* col) const {
     auto val = col->bulk_write(_writes).value();
     result::bulk_write res{std::move(val)};
-    stdx::optional<result::insert_many> result{{std::move(res), std::move(_inserted_ids)}};
+    stdx::optional<result::insert_many> result{{std::move(res), _inserted_ids.view()}};
     return result;
 };
 

src/mongocxx/insert_many_builder.hpp

@@ -15,6 +15,7 @@
 #pragma once
 
 #include <bsoncxx/stdx/optional.hpp>
+#include <bsoncxx/builder/basic/array.hpp>
 #include <mongocxx/bulk_write.hpp>
 #include <mongocxx/options/insert.hpp>
 #include <mongocxx/result/insert_many.hpp>
@@ -60,8 +61,7 @@ class MONGOCXX_API insert_many_builder {
 
    private:
     bulk_write _writes;
-    result::insert_many::id_map _inserted_ids;
-    std::size_t _index;
+    bsoncxx::builder::basic::array _inserted_ids;
 };
 
 MONGOCXX_INLINE_NAMESPACE_END

src/mongocxx/result/insert_many.cpp

@@ -14,14 +14,20 @@
 
 #include <mongocxx/result/insert_many.hpp>
 
+#include <bsoncxx/types/value.hpp>
+
 #include <mongocxx/config/private/prelude.hh>
 
 namespace mongocxx {
 MONGOCXX_INLINE_NAMESPACE_BEGIN
 namespace result {
 
-insert_many::insert_many(result::bulk_write result, insert_many::id_map inserted_ids)
-    : _result(std::move(result)), _generated_ids(std::move(inserted_ids)) {
+insert_many::insert_many(result::bulk_write result, bsoncxx::array::view inserted_ids)
+    : _result(std::move(result)), _inserted_ids_owned(inserted_ids) {
+    std::size_t index = 0;
+    for (auto&& ele : _inserted_ids_owned.view()) {
+        _inserted_ids.emplace(index++, ele.get_document().value["_id"]);
+    }
 }
 
 const result::bulk_write& insert_many::result() const {
@@ -33,7 +39,7 @@ std::int32_t insert_many::inserted_count() const {
 }
 
 insert_many::id_map insert_many::inserted_ids() {
-    return _generated_ids;
+    return _inserted_ids;
 }
 
 }  // namespace result

src/mongocxx/result/insert_many.hpp

@@ -17,6 +17,7 @@
 #include <cstdint>
 #include <map>
 
+#include <bsoncxx/array/value.hpp>
 #include <bsoncxx/types.hpp>
 #include <mongocxx/result/bulk_write.hpp>
 
@@ -55,18 +56,26 @@ class MONGOCXX_API insert_many {
     ///
     /// Gets the _ids of the inserted documents.
     ///
-    /// @return The values of the _id field for inserted documents.
+    /// @note The returned id_map must not be accessed after the result::insert_many object is
+    /// destroyed.
+    /// @return Map of the index of the operation to the _id of the inserted document.
     ///
     id_map inserted_ids();
 
    private:
     friend collection;
     friend insert_many_builder;
 
-    MONGOCXX_PRIVATE insert_many(result::bulk_write result, id_map inserted_ids);
+    MONGOCXX_PRIVATE insert_many(result::bulk_write result, bsoncxx::array::view inserted_ids);
 
     result::bulk_write _result;
-    id_map _generated_ids;
+
+    // Array containing documents with the values of the _id field for the inserted documents. This
+    // array is in the following format: [{"_id": ...}, {"_id": ...}, ...].
+    bsoncxx::array::value _inserted_ids_owned;
+
+    // Points into _inserted_ids_owned.
+    id_map _inserted_ids;
 };
 
 }  // namespace result

src/mongocxx/test/collection.cpp

@@ -147,6 +147,41 @@ TEST_CASE("CRUD functionality", "[driver::collection]") {
         REQUIRE(i == 4);
     }
 
+    SECTION("insert_many returns correct result object", "[collection]") {
+        document b1;
+        document b2;
+
+        b1 << "_id"
+           << "foo"
+           << "x" << 1;
+        b2 << "x" << 2;
+
+        std::vector<bsoncxx::document::view> docs{};
+        docs.push_back(b1.view());
+        docs.push_back(b2.view());
+
+        auto result = coll.insert_many(docs);
+
+        REQUIRE(result);
+
+        // Verify result->result() is correct:
+        REQUIRE(result->result().inserted_count() == 2);
+
+        // Verify result->inserted_count() is correct:
+        REQUIRE(result->inserted_count() == 2);
+
+        // Verify result->inserted_ids() is correct:
+        auto id_map = result->inserted_ids();
+        REQUIRE(id_map[0].type() == bsoncxx::type::k_utf8);
+        REQUIRE(id_map[0].get_utf8().value == stdx::string_view{"foo"});
+        REQUIRE(id_map[1].type() == bsoncxx::type::k_oid);
+        auto second_inserted_doc = coll.find_one(document{} << "x" << 2 << finalize);
+        REQUIRE(second_inserted_doc);
+        REQUIRE(second_inserted_doc->view()["_id"]);
+        REQUIRE(second_inserted_doc->view()["_id"].type() == bsoncxx::type::k_oid);
+        REQUIRE(id_map[1].get_oid().value == second_inserted_doc->view()["_id"].get_oid().value);
+    }
+
     SECTION("insert and update single document", "[collection]") {
         auto b1 = document{} << "_id" << 1 << finalize;