Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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();
Expand Down
35 changes: 13 additions & 22 deletions src/mongocxx/test/spec/gridfs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand All @@ -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 =
Expand All @@ -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>"
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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());
Expand Down