Skip to content

CXX-1817 Rename get_utf8() to get_string() to make it more user-friendly #721

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.

Sign up for GitHub

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 6 commits into from
Sep 14, 2020
Merged

CXX-1817 Rename get_utf8() to get_string() to make it more user-friendly #721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3f47436
CXX-1817 Rename get_utf8() to get_string() to make it more user-friendly
rayangler Sep 9, 2020
f8008f8
Begin deprecation of get_utf8()
rayangler Sep 11, 2020
2693be0
Replace instances of get_utf8() and suppress some deprecation warnings
rayangler Sep 11, 2020
42cc656
Replace get_utf8() with get_string() in examples
rayangler Sep 11, 2020
b104fa7
Lint files
rayangler Sep 11, 2020
eb722a6
Fix missing deprecation warning suppression
rayangler Sep 11, 2020
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
2 changes: 1 addition & 1 deletion docs/content/mongocxx-v3/tutorial.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ bsoncxx::document::element element = view["name"];
if(element.type() != bsoncxx::type::k_utf8) {
// Error
}
std::string name = element.get_utf8().value.to_string();
std::string name = element.get_string().value.to_string();
```

If the value in the `name` field is not a string and you do not
Expand Down
2 changes: 1 addition & 1 deletion examples/bsoncxx/view_and_value.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ int main(int, char**) {
array::view subarr{ele.get_array().value};
for (array::element ele : subarr) {
std::cout << "array element: "
<< bsoncxx::string::to_string(ele.get_utf8().value) << std::endl;
<< bsoncxx::string::to_string(ele.get_string().value) << std::endl;
}
break;
}
Expand Down
2 changes: 1 addition & 1 deletion examples/mongocxx/change_streams.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ std::string get_server_version(const mongocxx::client& client) {
server_status.append(bsoncxx::builder::basic::kvp("serverStatus", 1));
bsoncxx::document::value output = client["test"].run_command(server_status.extract());

return bsoncxx::string::to_string(output.view()["version"].get_utf8().value);
return bsoncxx::string::to_string(output.view()["version"].get_string().value);
}

void watch_until(const mongocxx::client& client,
Expand Down
2 changes: 1 addition & 1 deletion examples/mongocxx/get_values_from_documents.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ void iterate_messagelist(const bsoncxx::document::element& ele) {
std::cout << "status: " << status.get_int32().value << std::endl;
}
if (msg && msg.type() == type::k_utf8) {
std::cout << "msg: " << msg.get_utf8().value << std::endl;
std::cout << "msg: " << msg.get_string().value << std::endl;
}
} else {
std::cout << "Message is not a document" << std::endl;
Expand Down
2 changes: 1 addition & 1 deletion examples/mongocxx/mongodb.com/aggregation_examples.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ std::string get_server_version(const client& client) {
server_status.append(bsoncxx::builder::basic::kvp("serverStatus", 1));
bsoncxx::document::value output = client["test"].run_command(server_status.extract());

return bsoncxx::string::to_string(output.view()["version"].get_utf8().value);
return bsoncxx::string::to_string(output.view()["version"].get_string().value);
}

void aggregation_examples(const mongocxx::client& client, const mongocxx::database& db) {
Expand Down
8 changes: 4 additions & 4 deletions examples/mongocxx/mongodb.com/documentation_examples.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1036,11 +1036,11 @@ void update_examples(mongocxx::database db) {
// End Example 52

for (auto&& document : db["inventory"].find(make_document(kvp("item", "paper")))) {
if (document["size"].get_document().value["uom"].get_utf8().value !=
if (document["size"].get_document().value["uom"].get_string().value !=
bsoncxx::stdx::string_view{"cm"}) {
throw std::logic_error("error in example 52");
}
if (document["status"].get_utf8().value != bsoncxx::stdx::string_view{"P"}) {
if (document["status"].get_string().value != bsoncxx::stdx::string_view{"P"}) {
throw std::logic_error("error in example 52");
}
check_has_field(document, "lastModified", 52);
Expand All @@ -1060,11 +1060,11 @@ void update_examples(mongocxx::database db) {

for (auto&& document :
db["inventory"].find(make_document(kvp("qty", make_document(kvp("$lt", 50)))))) {
if (document["size"].get_document().value["uom"].get_utf8().value !=
if (document["size"].get_document().value["uom"].get_string().value !=
bsoncxx::stdx::string_view{"in"}) {
throw std::logic_error("error in example 53");
}
if (document["status"].get_utf8().value != bsoncxx::stdx::string_view{"P"}) {
if (document["status"].get_string().value != bsoncxx::stdx::string_view{"P"}) {
throw std::logic_error("error in example 53");
}
check_has_field(document, "lastModified", 53);
Expand Down
1 change: 1 addition & 0 deletions src/bsoncxx/array/element.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class BSONCXX_API element : private document::element {

using document::element::get_double;
using document::element::get_utf8;
using document::element::get_string;
using document::element::get_document;
using document::element::get_array;
using document::element::get_binary;
Expand Down
4 changes: 4 additions & 0 deletions src/bsoncxx/builder/core.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <bsoncxx/private/itoa.hh>
#include <bsoncxx/private/libbson.hh>
#include <bsoncxx/private/stack.hh>
#include <bsoncxx/private/suppress_deprecation_warnings.hh>
#include <bsoncxx/stdx/make_unique.hpp>
#include <bsoncxx/stdx/string_view.hpp>
#include <bsoncxx/string/to_string.hpp>
Expand Down Expand Up @@ -646,12 +647,15 @@ core& core::concatenate(const bsoncxx::document::view& view) {

core& core::append(const bsoncxx::types::bson_value::view& value) {
switch (static_cast<int>(value.type())) {
// CXX-1817; deprecation warning suppressed for get_utf8()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

BSONCXX_SUPPRESS_DEPRECATION_WARNINGS_BEGIN
#define BSONCXX_ENUM(type, val) \
case val: \
append(value.get_##type()); \
break;
#include <bsoncxx/enums/type.hpp>
#undef BSONCXX_ENUM
BSONCXX_SUPPRESS_DEPRECATION_WARNINGS_END
}

return *this;
Expand Down
12 changes: 12 additions & 0 deletions src/bsoncxx/document/element.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <bsoncxx/exception/exception.hpp>
#include <bsoncxx/json.hpp>
#include <bsoncxx/private/libbson.hh>
#include <bsoncxx/private/suppress_deprecation_warnings.hh>
#include <bsoncxx/types.hpp>
#include <bsoncxx/types/bson_value/value.hpp>
#include <bsoncxx/types/bson_value/view.hpp>
Expand Down Expand Up @@ -79,21 +80,32 @@ stdx::string_view element::key() const {
return stdx::string_view{key};
}

// CXX-1817; deprecation warning suppressed for get_utf8()
BSONCXX_SUPPRESS_DEPRECATION_WARNINGS_BEGIN
#define BSONCXX_ENUM(name, val) \
types::b_##name element::get_##name() const { \
types::bson_value::view v{_raw, _length, _offset, _keylen}; \
return v.get_##name(); \
}
#include <bsoncxx/enums/type.hpp>
#undef BSONCXX_ENUM
BSONCXX_SUPPRESS_DEPRECATION_WARNINGS_END

types::b_utf8 element::get_string() const {
types::bson_value::view v{_raw, _length, _offset, _keylen};
return v.get_string();
}

types::bson_value::view element::get_value() const {
switch (static_cast<int>(type())) {
// CXX-1817; deprecation warning suppressed for get_utf8()
BSONCXX_SUPPRESS_DEPRECATION_WARNINGS_BEGIN
#define BSONCXX_ENUM(type, val) \
case val: \
return types::bson_value::view{get_##type()};
#include <bsoncxx/enums/type.hpp>
#undef BSONCXX_ENUM
BSONCXX_SUPPRESS_DEPRECATION_WARNINGS_END
}

BSONCXX_UNREACHABLE;
Expand Down
14 changes: 13 additions & 1 deletion src/bsoncxx/document/element.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,23 @@ class BSONCXX_API element {
///
/// Getter for elements of the b_utf8 type.
///
/// @deprecated use document::element::get_string() instead.
///
/// @throws bsoncxx::exception if this element is not a b_utf8.
///
/// @return the element's value.
///
BSONCXX_DEPRECATED types::b_utf8 get_utf8() const;

///
/// Getter for elements of the b_utf8, or string, type. This function acts as a
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spirit of the requested change is to make it easier for users to find out how to get strings from BSON documents.

There are other types / enums with utf8 in the name:

  1. The bsoncxx::b_utf8 struct
  2. The k_utf8 value in the bsoncxx::type enum (generated by including src/bsoncxx/enums/type.hpp)
  3. The k_need_element_type_k_utf8 and k_cannot_append_utf8 error codes (also generated by including src/bsoncxx/enums/type.hpp)

We could deprecate (1) in favor of a newly named bsoncxx::b_string struct. But I'm not aware of a portable C++11 way to deprecate enum values for (2) and (3).

I think we should do one of two things:

  1. Deprecate the b_utf8 struct for a new b_string equivalent. And add *_k_string equivalents for the error codes and enum value. Otherwise we'll have element::get_utf8() deprecated, but users will still be required to use types / error codes with utf8. The error codes and enum value would need to be equal to avoid a backwards breaking change
  2. Leave get_utf8() undeprecated and just have get_string as an alias.

I have a slight preference for (2). My rationale is that it is extra work for users to move from deprecated to undeprecated API. Having the get_string helper still makes it easier for new users to find the API they want. And we can always choose to deprecate it later if need be.

Happy to chat about this further in slack/zoom.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After discussion, we'll do option 1, but as follow-up work in CXX-2109.

/// wrapper for get_utf8().
///
/// @throws bsoncxx::exception if this element is not a b_utf8.
///
/// @return the element's value.
///
types::b_utf8 get_utf8() const;
types::b_utf8 get_string() const;

///
/// Getter for elements of the b_document type.
Expand Down
4 changes: 2 additions & 2 deletions src/bsoncxx/test/bson_builder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -813,7 +813,7 @@ TEST_CASE("core method chaining to build array works", "[bsoncxx::builder::core]

REQUIRE(std::distance(array_view.begin(), array_view.end()) == 3);
REQUIRE(array_view[0].type() == type::k_utf8);
REQUIRE(string::to_string(array_view[0].get_utf8().value) == "foo");
REQUIRE(string::to_string(array_view[0].get_string().value) == "foo");
REQUIRE(array_view[1].type() == type::k_int32);
REQUIRE(array_view[1].get_int32().value == 1);
REQUIRE(array_view[2].type() == type::k_bool);
Expand Down Expand Up @@ -1198,7 +1198,7 @@ TEST_CASE("builder::basic::make_array works", "[bsoncxx::builder::basic::make_ar

REQUIRE(std::distance(array_view.begin(), array_view.end()) == 3);
REQUIRE(array_view[0].type() == type::k_utf8);
REQUIRE(string::to_string(array_view[0].get_utf8().value) == "foo");
REQUIRE(string::to_string(array_view[0].get_string().value) == "foo");
REQUIRE(array_view[1].type() == type::k_int32);
REQUIRE(array_view[1].get_int32().value == 1);
REQUIRE(array_view[2].type() == type::k_bool);
Expand Down
1 change: 1 addition & 0 deletions src/bsoncxx/test/bson_get_values.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#include <string>
#include <utility>

#include <bsoncxx/builder/basic/array.hpp>
Expand Down
2 changes: 1 addition & 1 deletion src/bsoncxx/test/bson_types.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ TEST_CASE("bson_value::view with b_double", "[bsoncxx::types::bson_value::view]"

TEST_CASE("bson_value::view with b_utf8", "[bsoncxx::types::bson_value::view]") {
b_utf8 utf8_val{"hello"};
REQUIRE(bson_value::view{utf8_val}.get_utf8() == utf8_val);
REQUIRE(bson_value::view{utf8_val}.get_string() == utf8_val);
}

TEST_CASE("bson_value::view with b_document", "[bsoncxx::types::bson_value::view]") {
Expand Down
14 changes: 14 additions & 0 deletions src/bsoncxx/types/bson_value/view.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <bsoncxx/exception/exception.hpp>
#include <bsoncxx/json.hpp>
#include <bsoncxx/private/libbson.hh>
#include <bsoncxx/private/suppress_deprecation_warnings.hh>
#include <bsoncxx/types/private/convert.hh>

#include <bsoncxx/config/private/prelude.hh>
Expand Down Expand Up @@ -66,12 +67,15 @@ view::view() noexcept : view(nullptr) {}

view::view(const view& rhs) noexcept {
switch (static_cast<int>(rhs._type)) {
// CXX-1817; deprecation warning suppressed for get_utf8()
BSONCXX_SUPPRESS_DEPRECATION_WARNINGS_BEGIN
#define BSONCXX_ENUM(type, val) \
case val: \
new (&_b_##type) b_##type(rhs.get_##type()); \
break;
#include <bsoncxx/enums/type.hpp>
#undef BSONCXX_ENUM
BSONCXX_SUPPRESS_DEPRECATION_WARNINGS_END
}

_type = rhs._type;
Expand All @@ -85,12 +89,15 @@ view& view::operator=(const view& rhs) noexcept {
destroy();

switch (static_cast<int>(rhs._type)) {
// CXX-1817; deprecation warning suppressed for get_utf8()
BSONCXX_SUPPRESS_DEPRECATION_WARNINGS_BEGIN
#define BSONCXX_ENUM(type, val) \
case val: \
new (&_b_##type) b_##type(rhs.get_##type()); \
break;
#include <bsoncxx/enums/type.hpp>
#undef BSONCXX_ENUM
BSONCXX_SUPPRESS_DEPRECATION_WARNINGS_END
}

_type = rhs._type;
Expand All @@ -113,6 +120,10 @@ bsoncxx::type view::type() const {
#include <bsoncxx/enums/type.hpp>
#undef BSONCXX_ENUM

const b_utf8& view::get_string() const {
return _b_utf8;
}

view::view(const std::uint8_t* raw,
std::uint32_t length,
std::uint32_t offset,
Expand Down Expand Up @@ -157,11 +168,14 @@ bool operator==(const view& lhs, const view& rhs) {
}

switch (static_cast<int>(lhs.type())) {
// CXX-1817; deprecation warning suppressed for get_utf8()
BSONCXX_SUPPRESS_DEPRECATION_WARNINGS_BEGIN
#define BSONCXX_ENUM(type, val) \
case val: \
return lhs.get_##type() == rhs.get_##type();
#include <bsoncxx/enums/type.hpp>
#undef BSONCXX_ENUM
BSONCXX_SUPPRESS_DEPRECATION_WARNINGS_END
}

// Silence compiler warnings about failing to return a value.
Expand Down
10 changes: 9 additions & 1 deletion src/bsoncxx/types/bson_value/view.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,15 @@ class BSONCXX_API view {
/// @warning
/// Calling the wrong get_<type> method will cause an exception to be thrown.
///
const b_utf8& get_utf8() const;
BSONCXX_DEPRECATED const b_utf8& get_utf8() const;

///
/// @return The underlying BSON UTF-8 string value.
///
/// @warning
/// Calling the wrong get_<type> method will cause an exception to be thrown.
///
const b_utf8& get_string() const;

///
/// @return The underlying BSON document value.
Expand Down
4 changes: 4 additions & 0 deletions src/bsoncxx/types/private/convert.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#pragma once

#include <bsoncxx/private/libbson.hh>
#include <bsoncxx/private/suppress_deprecation_warnings.hh>
#include <bsoncxx/types.hpp>
#include <bsoncxx/types/bson_value/view.hpp>
#include <cstdlib>
Expand Down Expand Up @@ -186,6 +187,8 @@ BSONCXX_INLINE void convert_to_libbson(const bsoncxx::types::b_array& arr, bson_
//
BSONCXX_INLINE void convert_to_libbson(bson_value_t* v, const class bson_value::view& bson_view) {
switch (bson_view.type()) {
// CXX-1817; deprecation warning suppressed for get_utf8()
BSONCXX_SUPPRESS_DEPRECATION_WARNINGS_BEGIN
#define BSONCXX_ENUM(name, val) \
case bsoncxx::type::k_##name: { \
auto value = bson_view.get_##name(); \
Expand All @@ -194,6 +197,7 @@ BSONCXX_INLINE void convert_to_libbson(bson_value_t* v, const class bson_value::
}
#include <bsoncxx/enums/type.hpp>
#undef BSONCXX_ENUM
BSONCXX_SUPPRESS_DEPRECATION_WARNINGS_END
default:
BSONCXX_UNREACHABLE;
}
Expand Down
5 changes: 3 additions & 2 deletions src/mongocxx/index_view.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ void index_view::drop_one(const bsoncxx::document::view_or_value& keys,
bsoncxx::document::view opts_view = index_options.view();

if (opts_view["name"]) {
drop_one(bsoncxx::string::to_string(opts_view["name"].get_utf8().value), options);
drop_one(bsoncxx::string::to_string(opts_view["name"].get_string().value), options);
} else {
drop_one(_get_impl().get_index_name_from_keys(keys), options);
}
Expand All @@ -108,7 +108,8 @@ void index_view::drop_one(const client_session& session,
bsoncxx::document::view opts_view = index_options.view();

if (opts_view["name"]) {
drop_one(session, bsoncxx::string::to_string(opts_view["name"].get_utf8().value), options);
drop_one(
session, bsoncxx::string::to_string(opts_view["name"].get_string().value), options);
} else {
drop_one(session, _get_impl().get_index_name_from_keys(keys), options);
}
Expand Down
4 changes: 2 additions & 2 deletions src/mongocxx/private/index_view.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,14 @@ class index_view::impl {
bsoncxx::document::view result_view = result.view();

if (result_view["note"] &&
bsoncxx::string::to_string(result_view["note"].get_utf8().value) ==
bsoncxx::string::to_string(result_view["note"].get_string().value) ==
"all indexes already exist") {
return bsoncxx::stdx::nullopt;
}

if (auto name = model.options()["name"]) {
return bsoncxx::stdx::make_optional(
bsoncxx::string::to_string(name.get_value().get_utf8().value));
bsoncxx::string::to_string(name.get_value().get_string().value));
}

return bsoncxx::stdx::make_optional(get_index_name_from_keys(model.keys()));
Expand Down
8 changes: 4 additions & 4 deletions src/mongocxx/test/change_streams.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ TEST_CASE("Mock streams and error-handling") {
REQUIRE(opts["batchSize"].get_int32() == batch_size);
REQUIRE(opts["maxAwaitTimeMS"].get_int64() == 4);
REQUIRE(opts["collation"].get_document().view() == collation);
REQUIRE(opts["fullDocument"].get_utf8().value ==
REQUIRE(opts["fullDocument"].get_string().value ==
bsoncxx::stdx::string_view{full_document});
REQUIRE(opts["resumeAfter"].get_document().view() == resume_after);
};
Expand Down Expand Up @@ -620,7 +620,7 @@ TEST_CASE("Watch a Collection", "[min36]") {

SECTION("Can receive it") {
auto it = *(x.begin());
REQUIRE(it["fullDocument"]["a"].get_utf8().value == stdx::string_view("b"));
REQUIRE(it["fullDocument"]["a"].get_string().value == stdx::string_view("b"));
}

SECTION("iterator equals itself") {
Expand All @@ -638,8 +638,8 @@ TEST_CASE("Watch a Collection", "[min36]") {
auto it = x.begin();
auto a = *it;
auto b = *it;
REQUIRE(a["fullDocument"]["a"].get_utf8().value == stdx::string_view("b"));
REQUIRE(b["fullDocument"]["a"].get_utf8().value == stdx::string_view("b"));
REQUIRE(a["fullDocument"]["a"].get_string().value == stdx::string_view("b"));
REQUIRE(b["fullDocument"]["a"].get_string().value == stdx::string_view("b"));
}

SECTION("Calling .begin multiple times doesn't advance state") {
Expand Down
Loading