CXX-3163 Remove workarounds for core::optional<T> (#1269)

Author: eramongodb

CHANGELOG.md

@@ -9,11 +9,16 @@ Changes prior to 3.9.0 are documented as [release notes on GitHub](https://githu
 
 ## 4.0.0 [Unreleased]
 
+## Added
+
+- Getter for the `start_at_operation_time` option in `mongocxx::v_noabi::options::change_stream`.
+
 ## Changed
 
 - CMake option `ENABLE_TESTS` is now `OFF` by default.
   - Set `ENABLE_TEST=ON` to re-enable building test targets.
   - Set `BUILD_TESTING=ON` to include test targets in the "all" target when `ENABLE_TESTS=ON` (since 3.9.0, `OFF` by default).
+- Layout of `mongocxx::v_noabi::options::change_stream` to support the new optional `start_at_operation_time` accessor.
 
 ## Removed
 

src/mongocxx/include/mongocxx/v_noabi/mongocxx/options/change_stream.hpp

@@ -275,6 +275,15 @@ class change_stream {
     MONGOCXX_ABI_EXPORT_CDECL(change_stream&)
     start_at_operation_time(bsoncxx::v_noabi::types::b_timestamp timestamp);
 
+    ///
+    /// The current start_at_operation_time setting.
+    ///
+    /// @return
+    ///   The current startAtOperationTime option.
+    ///
+    MONGOCXX_ABI_EXPORT_CDECL(const bsoncxx::stdx::optional<bsoncxx::v_noabi::types::b_timestamp>&)
+    start_at_operation_time() const;
+
    private:
     friend ::mongocxx::v_noabi::client;
     friend ::mongocxx::v_noabi::collection;
@@ -291,11 +300,7 @@ class change_stream {
     bsoncxx::v_noabi::stdx::optional<bsoncxx::v_noabi::document::view_or_value> _resume_after;
     bsoncxx::v_noabi::stdx::optional<bsoncxx::v_noabi::document::view_or_value> _start_after;
     bsoncxx::v_noabi::stdx::optional<std::chrono::milliseconds> _max_await_time;
-    // _start_at_operation_time is not wrapped in a bsoncxx::v_noabi::stdx::optional because of a
-    // longstanding bug in the MNMLSTC polyfill that has been fixed on master, but not in the latest
-    // release: https://github.com/mnmlstc/core/pull/23
-    bsoncxx::v_noabi::types::b_timestamp _start_at_operation_time = {};
-    bool _start_at_operation_time_set = false;
+    bsoncxx::v_noabi::stdx::optional<bsoncxx::v_noabi::types::b_timestamp> _start_at_operation_time;
 };
 
 }  // namespace options

src/mongocxx/lib/mongocxx/v_noabi/mongocxx/options/change_stream.cpp

@@ -112,11 +112,15 @@ const bsoncxx::v_noabi::stdx::optional<std::chrono::milliseconds>& change_stream
 
 change_stream& change_stream::start_at_operation_time(
     bsoncxx::v_noabi::types::b_timestamp timestamp) {
-    _start_at_operation_time = timestamp;
-    _start_at_operation_time_set = true;
+    _start_at_operation_time = std::move(timestamp);
     return *this;
 }
 
+const bsoncxx::stdx::optional<bsoncxx::v_noabi::types::b_timestamp>&
+change_stream::start_at_operation_time() const {
+    return _start_at_operation_time;
+}
+
 namespace {
 template <typename T>
 inline void append_if(bsoncxx::v_noabi::builder::basic::document& doc,
@@ -139,10 +143,7 @@ bsoncxx::v_noabi::document::value change_stream::as_bson() const {
     append_if(out, "batchSize", batch_size());
     append_if(out, "collation", collation());
     append_if(out, "comment", comment());
-    if (_start_at_operation_time_set) {
-        out.append(bsoncxx::v_noabi::builder::basic::kvp("startAtOperationTime",
-                                                         _start_at_operation_time));
-    }
+    append_if(out, "startAtOperationTime", start_at_operation_time());
 
     if (max_await_time()) {
         auto count = max_await_time().value().count();

src/mongocxx/test/spec/gridfs.cpp

@@ -46,18 +46,10 @@
 #include <mongocxx/test/client_helpers.hh>
 #include <mongocxx/test/spec/util.hh>
 
-namespace {
 using namespace bsoncxx;
 using namespace mongocxx;
 
-// This function is a workaround for clang 3.8 and mnmlstc/core, where `return
-// optional<item_t>{an_item_t};` fails to compile.
-bsoncxx::stdx::optional<test_util::item_t> make_optional(test_util::item_t item) {
-    bsoncxx::stdx::optional<test_util::item_t> option{};
-    option = item;
-
-    return option;
-}
+namespace {
 
 // Query the GridFS files collection and fetch the length of the file.
 //
@@ -80,20 +72,20 @@ std::int64_t get_length_of_gridfs_file(gridfs::bucket bucket, types::bson_value:
 bsoncxx::stdx::optional<test_util::item_t> transform_hex(test_util::item_t pair,
                                                          builder::basic::array* context) {
     if (!pair.first) {
-        return make_optional(pair);
+        return {pair};
     }
 
     auto key = *(pair.first);
     auto value = pair.second;
 
     if (bsoncxx::string::to_string(key) != "data" || value.type() != type::k_document) {
-        return make_optional(pair);
+        return {pair};
     }
 
     auto data = value.get_document().value;
 
     if (!data["$hex"] || data["$hex"].type() != type::k_string) {
-        return make_optional(pair);
+        return {pair};
     }
 
     std::basic_string<std::uint8_t> bytes =
@@ -106,8 +98,8 @@ bsoncxx::stdx::optional<test_util::item_t> transform_hex(test_util::item_t pair,
     auto view = context->view();
     auto length = std::distance(view.cbegin(), view.cend());
 
-    return make_optional(std::make_pair(bsoncxx::stdx::optional<bsoncxx::stdx::string_view>("data"),
-                                        view[static_cast<std::uint32_t>(length - 1)].get_value()));
+    return {std::make_pair(bsoncxx::stdx::optional<bsoncxx::stdx::string_view>("data"),
+                           view[static_cast<std::uint32_t>(length - 1)].get_value())};
 }
 
 // The GridFS spec specifies the expected binary data in the form of { $hex: "<hexadecimal string>"
@@ -120,21 +112,20 @@ document::value convert_hex_data_to_binary(document::view document) {
 bsoncxx::stdx::optional<test_util::item_t> convert_length_to_int64(test_util::item_t pair,
                                                                    builder::basic::array*) {
     if (!pair.first) {
-        return make_optional(pair);
+        return {pair};
     }
 
     auto key = *(pair.first);
     auto value = pair.second;
 
     if (bsoncxx::string::to_string(key) != "length" || value.type() != type::k_int32) {
-        return make_optional(pair);
+        return {pair};
     }
 
     types::b_int64 length = {value.get_int32()};
 
-    return make_optional(
-        std::make_pair(bsoncxx::stdx::optional<bsoncxx::stdx::string_view>("length"),
-                       types::bson_value::view{length}));
+    return {std::make_pair(bsoncxx::stdx::optional<bsoncxx::stdx::string_view>("length"),
+                           types::bson_value::view{length})};
 }
 
 void compare_collections(database db) {
@@ -306,7 +297,7 @@ void test_upload(database db,
             [id](test_util::item_t pair,
                  builder::basic::array* context) -> bsoncxx::stdx::optional<test_util::item_t> {
                 if (!pair.first) {
-                    return make_optional(pair);
+                    return {pair};
                 }
 
                 auto key = *(pair.first);
@@ -336,10 +327,10 @@ void test_upload(database db,
                 }
 
                 if (id_str != "*result") {
-                    return make_optional(pair);
+                    return {pair};
                 }
 
-                return make_optional(std::make_pair(pair.first, types::bson_value::view{id}));
+                return {std::make_pair(pair.first, types::bson_value::view{id})};
             });
 
         db.run_command(transformed_data.view());