From cfbff4dd0f408640130f478ce51755d6c3289835 Mon Sep 17 00:00:00 2001 From: Clyde Bazile Date: Thu, 9 Jul 2020 15:24:21 -0400 Subject: [PATCH 1/8] =?UTF-8?q?CXX-1998=20'CommitQuorum'=20option=20suppor?= =?UTF-8?q?t=20for=20'createIndexes=E2=80=99=20command=20on=20MongoDB=204.?= =?UTF-8?q?4'?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/mongocxx/options/index_view.cpp | 23 +++++++++++- src/mongocxx/options/index_view.hpp | 54 +++++++++++++++++++++++++++++ src/mongocxx/private/index_view.hh | 12 ++++--- src/mongocxx/test/index_view.cpp | 40 +++++++++++++++++++++ 4 files changed, 124 insertions(+), 5 deletions(-) diff --git a/src/mongocxx/options/index_view.cpp b/src/mongocxx/options/index_view.cpp index 31bf12dd3c..83f139a1c0 100644 --- a/src/mongocxx/options/index_view.cpp +++ b/src/mongocxx/options/index_view.cpp @@ -12,9 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include +#include +#include +#include #include -#include +using bsoncxx::builder::basic::kvp; +using bsoncxx::builder::basic::make_document; namespace mongocxx { MONGOCXX_INLINE_NAMESPACE_BEGIN @@ -30,6 +35,10 @@ const bsoncxx::stdx::optional& index_view::max_time() return _max_time; } +const stdx::optional index_view::commit_quorum() const { + return _commit_quorum; +} + index_view& index_view::max_time(std::chrono::milliseconds max_time) { _max_time = max_time; return *this; @@ -40,6 +49,18 @@ index_view& index_view::write_concern(mongocxx::write_concern write_concern) { return *this; } +index_view& index_view::commit_quorum(int commit_quorum) { + _commit_quorum = stdx::make_optional( + make_document(kvp("commitQuorum", bsoncxx::types::b_int32{commit_quorum}))); + return *this; +} + +index_view& index_view::commit_quorum(std::string commit_quorum) { + _commit_quorum = stdx::make_optional( + make_document(kvp("commitQuorum", commit_quorum))); + return *this; +} } // namespace options + MONGOCXX_INLINE_NAMESPACE_END } // namespace mongocxx diff --git a/src/mongocxx/options/index_view.hpp b/src/mongocxx/options/index_view.hpp index 19d8cda117..1322c3c522 100644 --- a/src/mongocxx/options/index_view.hpp +++ b/src/mongocxx/options/index_view.hpp @@ -84,9 +84,63 @@ class MONGOCXX_API index_view { /// const bsoncxx::stdx::optional& write_concern() const; + /// + /// Sets the commit quorum for this operation. + /// + /// This option may only be used with MongoDB version 4.4 or later. + /// + /// @param commit_quorum + /// Integer representing the minimum number of data-bearing voting replica set members (i.e. + /// commit quorum), including the primary, that must report a successful index build before + /// the primary marks the indexes as ready. A value of @c 0 disables quorum-voting behavior. + /// + /// @return + /// A reference to the object on which this member function is being called. This facilitates + /// method chaining. + /// + /// @see + /// https://docs.mongodb.com/master/reference/command/createIndexes + /// + index_view& commit_quorum(int commit_quorum); + + /// + /// Sets the commit quorum for this operation. + /// + /// The server-side default is "votingMembers". + /// + /// This option may only be used with MongoDB version 4.4 or later. + /// + /// @param commit_quorum + /// String representing the minimum number of data-bearing voting replica set members (i.e. + /// commit quorum), including the primary, that must report a successful index build before + /// the primary marks the indexes as ready. + /// + /// @return + /// A reference to the object on which this member function is being called. This facilitates + /// method chaining. + /// + /// @see + /// https://docs.mongodb.com/master/reference/command/createIndexes + /// + index_view& commit_quorum(std::string commit_quorum); + + /// + /// Gets the current commitQuorum setting. + /// + /// This option may only be used with MongoDB version 4.4 or later. + /// + /// @return + /// The current commitQuorum setting. + /// + /// @see + /// https://docs.mongodb.com/master/reference/command/createIndexes + /// + const stdx::optional commit_quorum() const; + private: bsoncxx::stdx::optional _max_time; bsoncxx::stdx::optional _write_concern; + bsoncxx::stdx::optional _commit_quorum; }; } // namespace options diff --git a/src/mongocxx/private/index_view.hh b/src/mongocxx/private/index_view.hh index 45a499af0e..2a3e32f8b1 100644 --- a/src/mongocxx/private/index_view.hh +++ b/src/mongocxx/private/index_view.hh @@ -14,13 +14,13 @@ #pragma once -#include - #include #include #include +#include #include #include +#include #include #include #include @@ -28,8 +28,8 @@ #include #include #include - -#include +#include +#include namespace mongocxx { MONGOCXX_INLINE_NAMESPACE_BEGIN @@ -134,6 +134,10 @@ class index_view::impl { opts_doc.append(bsoncxx::builder::concatenate_doc{session->_get_impl().to_document()}); } + if (options.commit_quorum()) { + opts_doc.append(concatenate(options.commit_quorum()->view())); + } + libbson::scoped_bson_t command_bson{command}; libbson::scoped_bson_t opts_bson{opts_doc.view()}; diff --git a/src/mongocxx/test/index_view.cpp b/src/mongocxx/test/index_view.cpp index 8cc7c86d25..ab61665eb0 100644 --- a/src/mongocxx/test/index_view.cpp +++ b/src/mongocxx/test/index_view.cpp @@ -172,6 +172,46 @@ TEST_CASE("create_one", "[index_view]") { REQUIRE_NOTHROW(indexes.create_one(keys1.view(), options.view())); REQUIRE_THROWS_AS(indexes.create_one(keys2.view(), options.view()), operation_exception); } + + SECTION("commitQuorum option") { + collection coll = db["index_view_create_one_commit_quorum"]; + coll.drop(); + coll.insert_one({}); // Ensure that the collection exists. + index_view indexes = coll.indexes(); + + auto key = make_document(kvp("a", 1)); + index_model model(key.view()); + options::index_view options; + + auto commit_quorum_regex = + Catch::Matches("(.*)commit( )?quorum(.*)", Catch::CaseSensitive::No); + + using namespace test_util; + if (compare_versions(get_server_version(mongodb_client), "4.4") < 0 || + get_topology(mongodb_client) == "single") { + return; + } + + SECTION("works with int") { + options.commit_quorum(1); + REQUIRE_NOTHROW(indexes.create_one(model, options)); + } + + SECTION("fails with invalid int") { + options.commit_quorum(-1); + REQUIRE_THROWS_WITH(indexes.create_one(model, options), commit_quorum_regex); + } + + SECTION("works with string") { + options.commit_quorum("majority"); + REQUIRE_NOTHROW(indexes.create_one(model, options)); + } + + SECTION("fails with invalid string") { + options.commit_quorum("bad_str"); + REQUIRE_THROWS_WITH(indexes.create_one(model, options), commit_quorum_regex); + } + } } TEST_CASE("create_many", "[index_view]") { From f4780f8e9bb5d98a1a029b26c2c2fba08735d92c Mon Sep 17 00:00:00 2001 From: Clyde Bazile Date: Tue, 14 Jul 2020 15:41:33 -0400 Subject: [PATCH 2/8] address comments --- src/mongocxx/options/index_view.hpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/mongocxx/options/index_view.hpp b/src/mongocxx/options/index_view.hpp index 1322c3c522..3b3d671e67 100644 --- a/src/mongocxx/options/index_view.hpp +++ b/src/mongocxx/options/index_view.hpp @@ -101,13 +101,11 @@ class MONGOCXX_API index_view { /// @see /// https://docs.mongodb.com/master/reference/command/createIndexes /// - index_view& commit_quorum(int commit_quorum); + index_view& commit_quorum(std::int32_t commit_quorum); /// /// Sets the commit quorum for this operation. /// - /// The server-side default is "votingMembers". - /// /// This option may only be used with MongoDB version 4.4 or later. /// /// @param commit_quorum From 43005fd9c1963158d41aef42eb22ce904ae39235 Mon Sep 17 00:00:00 2001 From: Clyde Bazile Date: Wed, 15 Jul 2020 12:15:42 -0400 Subject: [PATCH 3/8] throw err on unsupported wire version --- src/mongocxx/collection.cpp | 2 +- src/mongocxx/index_view.cpp | 5 +++-- src/mongocxx/index_view.hpp | 2 +- src/mongocxx/private/index_view.hh | 21 +++++++++++++++++++-- src/mongocxx/private/libmongoc_symbols.hh | 1 + src/mongocxx/test/index_view.cpp | 18 ++++++++++++------ 6 files changed, 37 insertions(+), 12 deletions(-) diff --git a/src/mongocxx/collection.cpp b/src/mongocxx/collection.cpp index 9fb857d47f..57d05db3ff 100644 --- a/src/mongocxx/collection.cpp +++ b/src/mongocxx/collection.cpp @@ -1282,7 +1282,7 @@ class change_stream collection::_watch(const client_session* session, } class index_view collection::indexes() { - return index_view{_get_impl().collection_t}; + return index_view{_get_impl().collection_t, _get_impl().client_impl->client_t}; } class bulk_write collection::_init_insert_many(const options::insert& options, diff --git a/src/mongocxx/index_view.cpp b/src/mongocxx/index_view.cpp index 65c952d817..777849a215 100644 --- a/src/mongocxx/index_view.cpp +++ b/src/mongocxx/index_view.cpp @@ -26,8 +26,9 @@ namespace mongocxx { MONGOCXX_INLINE_NAMESPACE_BEGIN -index_view::index_view(void* coll) - : _impl{stdx::make_unique(static_cast(coll))} {} +index_view::index_view(void* coll, void* client) + : _impl{stdx::make_unique(static_cast(coll), + static_cast(client))} {} index_view::index_view(index_view&&) noexcept = default; index_view& index_view::operator=(index_view&&) noexcept = default; diff --git a/src/mongocxx/index_view.hpp b/src/mongocxx/index_view.hpp index 13c6616b66..5a66f5b8e9 100644 --- a/src/mongocxx/index_view.hpp +++ b/src/mongocxx/index_view.hpp @@ -421,7 +421,7 @@ class MONGOCXX_API index_view { friend class collection; class MONGOCXX_PRIVATE impl; - MONGOCXX_PRIVATE index_view(void* coll); + MONGOCXX_PRIVATE index_view(void* coll, void* client); MONGOCXX_PRIVATE impl& _get_impl(); diff --git a/src/mongocxx/private/index_view.hh b/src/mongocxx/private/index_view.hh index 2a3e32f8b1..27bb971ec1 100644 --- a/src/mongocxx/private/index_view.hh +++ b/src/mongocxx/private/index_view.hh @@ -39,7 +39,8 @@ using bsoncxx::builder::basic::kvp; class index_view::impl { public: - impl(mongoc_collection_t* collection) : _coll{collection} {} + impl(mongoc_collection_t* collection, mongoc_client_t* client) + : _coll{collection}, _client{client} {} impl(const impl& i) = default; @@ -135,7 +136,22 @@ class index_view::impl { } if (options.commit_quorum()) { - opts_doc.append(concatenate(options.commit_quorum()->view())); + auto server_description = libmongoc::client_select_server( + _client, true /* for_writes */, nullptr /* read_prefs */, &error); + auto is_master = libmongoc::server_description_ismaster(server_description); + + bson_iter_t iter; + bson_iter_init_find(&iter, is_master, "maxWireVersion"); + int32_t max_wire_version = bson_iter_int32(&iter); + + if (max_wire_version < 9) { + throw logic_error{ + error_code::k_invalid_parameter, + "option 'commitQuorum' not available on the current server version"}; + } + + command = + make_document(concatenate(command), concatenate(options.commit_quorum()->view())); } libbson::scoped_bson_t command_bson{command}; @@ -223,6 +239,7 @@ class index_view::impl { } mongoc_collection_t* _coll; + mongoc_client_t* _client; }; MONGOCXX_INLINE_NAMESPACE_END } // namespace mongocxx diff --git a/src/mongocxx/private/libmongoc_symbols.hh b/src/mongocxx/private/libmongoc_symbols.hh index afef003ee1..1cd3f7480e 100644 --- a/src/mongocxx/private/libmongoc_symbols.hh +++ b/src/mongocxx/private/libmongoc_symbols.hh @@ -154,6 +154,7 @@ MONGOCXX_LIBMONGOC_SYMBOL(client_pool_push) MONGOCXX_LIBMONGOC_SYMBOL(client_pool_set_apm_callbacks) MONGOCXX_LIBMONGOC_SYMBOL(client_pool_try_pop) MONGOCXX_LIBMONGOC_SYMBOL(client_reset) +MONGOCXX_LIBMONGOC_SYMBOL(client_select_server); MONGOCXX_LIBMONGOC_SYMBOL(client_session_abort_transaction) MONGOCXX_LIBMONGOC_SYMBOL(client_session_advance_cluster_time) MONGOCXX_LIBMONGOC_SYMBOL(client_session_advance_operation_time) diff --git a/src/mongocxx/test/index_view.cpp b/src/mongocxx/test/index_view.cpp index ab61665eb0..d63d676ef0 100644 --- a/src/mongocxx/test/index_view.cpp +++ b/src/mongocxx/test/index_view.cpp @@ -187,14 +187,16 @@ TEST_CASE("create_one", "[index_view]") { Catch::Matches("(.*)commit( )?quorum(.*)", Catch::CaseSensitive::No); using namespace test_util; - if (compare_versions(get_server_version(mongodb_client), "4.4") < 0 || - get_topology(mongodb_client) == "single") { - return; - } + bool is_supported = compare_versions(get_server_version(mongodb_client), "4.4") >= 0 && + get_topology(mongodb_client) != "single"; SECTION("works with int") { options.commit_quorum(1); - REQUIRE_NOTHROW(indexes.create_one(model, options)); + if (is_supported) { + REQUIRE_NOTHROW(indexes.create_one(model, options)); + } else { + REQUIRE_THROWS_WITH(indexes.create_one(model, options), commit_quorum_regex); + } } SECTION("fails with invalid int") { @@ -204,7 +206,11 @@ TEST_CASE("create_one", "[index_view]") { SECTION("works with string") { options.commit_quorum("majority"); - REQUIRE_NOTHROW(indexes.create_one(model, options)); + if (is_supported) { + REQUIRE_NOTHROW(indexes.create_one(model, options)); + } else { + REQUIRE_THROWS_WITH(indexes.create_one(model, options), commit_quorum_regex); + } } SECTION("fails with invalid string") { From a4630ce6685a6b86ad91c8cbbaed871df4d62ed5 Mon Sep 17 00:00:00 2001 From: Clyde Bazile Date: Wed, 15 Jul 2020 13:27:03 -0400 Subject: [PATCH 4/8] remove unused headers --- src/mongocxx/private/index_view.hh | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/mongocxx/private/index_view.hh b/src/mongocxx/private/index_view.hh index 27bb971ec1..ce8f4b9b5c 100644 --- a/src/mongocxx/private/index_view.hh +++ b/src/mongocxx/private/index_view.hh @@ -17,7 +17,6 @@ #include #include #include -#include #include #include #include @@ -28,7 +27,6 @@ #include #include #include -#include #include namespace mongocxx { From cd7f158c73d0a7398984117cf31104efce000b64 Mon Sep 17 00:00:00 2001 From: Clyde Bazile Date: Wed, 15 Jul 2020 15:55:17 -0400 Subject: [PATCH 5/8] address comments --- src/mongocxx/private/index_view.hh | 27 ++++++++++++++++------- src/mongocxx/private/libmongoc_symbols.hh | 2 +- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/mongocxx/private/index_view.hh b/src/mongocxx/private/index_view.hh index ce8f4b9b5c..a5c861c6da 100644 --- a/src/mongocxx/private/index_view.hh +++ b/src/mongocxx/private/index_view.hh @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -134,16 +135,17 @@ class index_view::impl { } if (options.commit_quorum()) { - auto server_description = libmongoc::client_select_server( - _client, true /* for_writes */, nullptr /* read_prefs */, &error); - auto is_master = libmongoc::server_description_ismaster(server_description); + auto server_description = scoped_server_description(libmongoc::client_select_server( + _client, true /* for_writes */, nullptr /* read_prefs */, &error)); + if (!server_description.sd) + throw_exception(error); - bson_iter_t iter; - bson_iter_init_find(&iter, is_master, "maxWireVersion"); - int32_t max_wire_version = bson_iter_int32(&iter); + auto is_master = libmongoc::server_description_ismaster(server_description.sd); - if (max_wire_version < 9) { - throw logic_error{ + bson_iter_t iter; + if (!bson_iter_init_find(&iter, is_master, "maxWireVersion") || + bson_iter_int32(&iter) < 9) { + throw write_exception{ error_code::k_invalid_parameter, "option 'commitQuorum' not available on the current server version"}; } @@ -238,6 +240,15 @@ class index_view::impl { mongoc_collection_t* _coll; mongoc_client_t* _client; + + class scoped_server_description { + public: + explicit scoped_server_description(mongoc_server_description_t* sd) : sd(sd) {} + ~scoped_server_description() { + mongoc_server_description_destroy(sd); + } + mongoc_server_description_t* sd; + }; }; MONGOCXX_INLINE_NAMESPACE_END } // namespace mongocxx diff --git a/src/mongocxx/private/libmongoc_symbols.hh b/src/mongocxx/private/libmongoc_symbols.hh index 1cd3f7480e..fe1ac77ae0 100644 --- a/src/mongocxx/private/libmongoc_symbols.hh +++ b/src/mongocxx/private/libmongoc_symbols.hh @@ -154,7 +154,7 @@ MONGOCXX_LIBMONGOC_SYMBOL(client_pool_push) MONGOCXX_LIBMONGOC_SYMBOL(client_pool_set_apm_callbacks) MONGOCXX_LIBMONGOC_SYMBOL(client_pool_try_pop) MONGOCXX_LIBMONGOC_SYMBOL(client_reset) -MONGOCXX_LIBMONGOC_SYMBOL(client_select_server); +MONGOCXX_LIBMONGOC_SYMBOL(client_select_server) MONGOCXX_LIBMONGOC_SYMBOL(client_session_abort_transaction) MONGOCXX_LIBMONGOC_SYMBOL(client_session_advance_cluster_time) MONGOCXX_LIBMONGOC_SYMBOL(client_session_advance_operation_time) From bb8c3589ea85d31f06ff4d9d7c0273d7b7164541 Mon Sep 17 00:00:00 2001 From: Clyde Bazile Date: Thu, 16 Jul 2020 21:13:21 -0400 Subject: [PATCH 6/8] use wire version --- src/mongocxx/test/index_view.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/mongocxx/test/index_view.cpp b/src/mongocxx/test/index_view.cpp index d63d676ef0..14f4274b58 100644 --- a/src/mongocxx/test/index_view.cpp +++ b/src/mongocxx/test/index_view.cpp @@ -187,8 +187,9 @@ TEST_CASE("create_one", "[index_view]") { Catch::Matches("(.*)commit( )?quorum(.*)", Catch::CaseSensitive::No); using namespace test_util; - bool is_supported = compare_versions(get_server_version(mongodb_client), "4.4") >= 0 && - get_topology(mongodb_client) != "single"; + bool is_supported = + get_max_wire_version(mongodb_client) >= 9 && get_topology(mongodb_client) != "single"; + CAPTURE(is_supported); SECTION("works with int") { options.commit_quorum(1); From 63e5a701b6701245b9c5e100c6cf216682a38669 Mon Sep 17 00:00:00 2001 From: Clyde Bazile Date: Fri, 17 Jul 2020 13:04:22 -0400 Subject: [PATCH 7/8] skip single typologies --- src/mongocxx/test/index_view.cpp | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/src/mongocxx/test/index_view.cpp b/src/mongocxx/test/index_view.cpp index 14f4274b58..8d722ee2a7 100644 --- a/src/mongocxx/test/index_view.cpp +++ b/src/mongocxx/test/index_view.cpp @@ -174,6 +174,9 @@ TEST_CASE("create_one", "[index_view]") { } SECTION("commitQuorum option") { + if (test_util::get_topology(mongodb_client) == "single") + return; + collection coll = db["index_view_create_one_commit_quorum"]; coll.drop(); coll.insert_one({}); // Ensure that the collection exists. @@ -186,9 +189,7 @@ TEST_CASE("create_one", "[index_view]") { auto commit_quorum_regex = Catch::Matches("(.*)commit( )?quorum(.*)", Catch::CaseSensitive::No); - using namespace test_util; - bool is_supported = - get_max_wire_version(mongodb_client) >= 9 && get_topology(mongodb_client) != "single"; + bool is_supported = test_util::get_max_wire_version(mongodb_client) >= 9; CAPTURE(is_supported); SECTION("works with int") { @@ -200,11 +201,6 @@ TEST_CASE("create_one", "[index_view]") { } } - SECTION("fails with invalid int") { - options.commit_quorum(-1); - REQUIRE_THROWS_WITH(indexes.create_one(model, options), commit_quorum_regex); - } - SECTION("works with string") { options.commit_quorum("majority"); if (is_supported) { @@ -213,11 +209,6 @@ TEST_CASE("create_one", "[index_view]") { REQUIRE_THROWS_WITH(indexes.create_one(model, options), commit_quorum_regex); } } - - SECTION("fails with invalid string") { - options.commit_quorum("bad_str"); - REQUIRE_THROWS_WITH(indexes.create_one(model, options), commit_quorum_regex); - } } } From 64f220aebc0937d8dbd6e8b4a177561fc87bb7bb Mon Sep 17 00:00:00 2001 From: Clyde Bazile Date: Fri, 17 Jul 2020 14:15:55 -0400 Subject: [PATCH 8/8] add warning --- src/mongocxx/test/index_view.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/mongocxx/test/index_view.cpp b/src/mongocxx/test/index_view.cpp index 8d722ee2a7..7752d361ca 100644 --- a/src/mongocxx/test/index_view.cpp +++ b/src/mongocxx/test/index_view.cpp @@ -174,8 +174,10 @@ TEST_CASE("create_one", "[index_view]") { } SECTION("commitQuorum option") { - if (test_util::get_topology(mongodb_client) == "single") + if (test_util::get_topology(mongodb_client) == "single") { + WARN("skip: commitQuorum option requires a replica set"); return; + } collection coll = db["index_view_create_one_commit_quorum"]; coll.drop();