Skip to content

Commit 16777df

Browse files
joshbsiegeljoshbsiegel
authored
CXX-2120 handle nonexistent document fields (#984)
1 parent 1567475 commit 16777df

File tree

7 files changed

+89
-19
lines changed

7 files changed

+89
-19
lines changed

‎src/bsoncxx/array/element.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ element::element(const std::uint8_t* raw,
3131
std::uint32_t keylen)
3232
: document::element(raw, length, offset, keylen) {}
3333

34+
element::element(const stdx::string_view key) : document::element(key) {}
35+
3436
bool BSONCXX_CALL operator==(const element& elem, const types::bson_value::view& v) {
3537
return elem.get_value() == v;
3638
}

‎src/bsoncxx/array/element.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ class BSONCXX_API element : private document::element {
8787
std::uint32_t length,
8888
std::uint32_t offset,
8989
std::uint32_t keylen);
90+
91+
BSONCXX_PRIVATE explicit element(const stdx::string_view key);
9092
};
9193

9294
///

‎src/bsoncxx/array/view.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,15 +118,15 @@ view::const_iterator view::find(std::uint32_t i) const {
118118
bson_iter_t iter;
119119

120120
if (!bson_init_static(&b, data(), length())) {
121-
return cend();
121+
return const_iterator(element(key.c_str()));
122122
}
123123

124124
if (!bson_iter_init(&iter, &b)) {
125-
return cend();
125+
return const_iterator(element(key.c_str()));
126126
}
127127

128128
if (!bson_iter_init_find(&iter, &b, key.c_str())) {
129-
return cend();
129+
return const_iterator(element(key.c_str()));
130130
}
131131

132132
return const_iterator(element(data(),

‎src/bsoncxx/document/element.cpp

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ element::element(const std::uint8_t* raw,
4141
std::uint32_t keylen)
4242
: _raw(raw), _length(length), _offset(offset), _keylen(keylen) {}
4343

44+
element::element(const stdx::string_view key)
45+
: _raw(nullptr), _length(0), _offset(0), _keylen(0), _key(key) {}
46+
4447
const std::uint8_t* element::raw() const {
4548
return _raw;
4649
}
@@ -58,8 +61,10 @@ std::uint32_t element::keylen() const {
5861

5962
bsoncxx::type element::type() const {
6063
if (_raw == nullptr) {
61-
throw bsoncxx::exception{error_code::k_unset_element,
62-
"cannot return the type of uninitialized element"};
64+
throw bsoncxx::exception{
65+
error_code::k_unset_element,
66+
"cannot return the type of uninitialized element" +
67+
std::string(_key ? " with key \"" + std::string(_key.value().data()) + "\"" : "")};
6368
}
6469

6570
BSONCXX_CITER;
@@ -68,8 +73,10 @@ bsoncxx::type element::type() const {
6873

6974
stdx::string_view element::key() const {
7075
if (_raw == nullptr) {
71-
throw bsoncxx::exception{error_code::k_unset_element,
72-
"cannot return the key from an uninitialized element"};
76+
throw bsoncxx::exception{
77+
error_code::k_unset_element,
78+
"cannot return the key from an uninitialized element" +
79+
std::string(_key ? " with key \"" + std::string(_key.value().data()) + "\"" : "")};
7380
}
7481

7582
BSONCXX_CITER;
@@ -79,22 +86,27 @@ stdx::string_view element::key() const {
7986
return stdx::string_view{key};
8087
}
8188

82-
#define BSONCXX_ENUM(name, val) \
83-
types::b_##name element::get_##name() const { \
84-
if (_raw == nullptr) { \
85-
throw bsoncxx::exception{error_code::k_unset_element, \
86-
"cannot get " #name " from an uninitialized element"}; \
87-
} \
88-
types::bson_value::view v{_raw, _length, _offset, _keylen}; \
89-
return v.get_##name(); \
89+
#define BSONCXX_ENUM(name, val) \
90+
types::b_##name element::get_##name() const { \
91+
if (_raw == nullptr) { \
92+
throw bsoncxx::exception{ \
93+
error_code::k_unset_element, \
94+
"cannot get " #name " from an uninitialized element" + \
95+
std::string(_key ? " with key \"" + std::string(_key.value().data()) + "\"" \
96+
: "")}; \
97+
} \
98+
types::bson_value::view v{_raw, _length, _offset, _keylen}; \
99+
return v.get_##name(); \
90100
}
91101
#include <bsoncxx/enums/type.hpp>
92102
#undef BSONCXX_ENUM
93103

94104
types::b_string element::get_utf8() const {
95105
if (_raw == nullptr) {
96-
throw bsoncxx::exception{error_code::k_unset_element,
97-
"cannot get string from an uninitialized element"};
106+
throw bsoncxx::exception{
107+
error_code::k_unset_element,
108+
"cannot get string from an uninitialized element" +
109+
std::string(_key ? " with key \"" + std::string(_key.value().data()) + "\"" : "")};
98110
}
99111

100112
types::bson_value::view v{_raw, _length, _offset, _keylen};

‎src/bsoncxx/document/element.hpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <cstddef>
1818
#include <cstdint>
1919

20+
#include <bsoncxx/stdx/optional.hpp>
2021
#include <bsoncxx/stdx/string_view.hpp>
2122

2223
#include <bsoncxx/config/prelude.hpp>
@@ -401,13 +402,20 @@ class BSONCXX_API element {
401402
std::uint32_t offset,
402403
std::uint32_t keylen);
403404

405+
// Construct an invalid element with a key. Useful for exceptions.
406+
BSONCXX_PRIVATE explicit element(const stdx::string_view key);
407+
404408
friend class view;
405409
friend class array::element;
406410

407411
const std::uint8_t* _raw;
408412
std::uint32_t _length;
409413
std::uint32_t _offset;
410414
std::uint32_t _keylen;
415+
// _key will only exist when a caller attempts to find a key in the BSON but is unsuccessful.
416+
// The key is stored for a more helpful error message if the user tries to access the value of
417+
// a key that does not exist.
418+
stdx::optional<stdx::string_view> _key;
411419
};
412420

413421
///

‎src/bsoncxx/document/view.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,8 @@ view::const_iterator view::end() const {
112112
view::const_iterator view::find(stdx::string_view key) const {
113113
bson_t b;
114114
if (!bson_init_static(&b, _data, _length)) {
115-
return cend();
115+
// return invalid element with key to provide more helpful exception message.
116+
return const_iterator(element(key));
116117
}
117118

118119
bson_iter_t iter;
@@ -131,7 +132,9 @@ view::const_iterator view::find(stdx::string_view key) const {
131132
}
132133

133134
if (!bson_iter_init_find_w_len(&iter, &b, key.data(), static_cast<int>(key.size()))) {
134-
return cend();
135+
// returning `cend()` returns an element without a key or value.
136+
// return invalid element with key to provide more helpful exception
137+
return const_iterator(element(key));
135138
}
136139

137140
return const_iterator(element(

‎src/bsoncxx/test/bson_types.cpp

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,4 +366,47 @@ TEST_CASE("bson_value::view with inequality for non-value and value",
366366
b_int64 int64_val{100};
367367
REQUIRE(int64_val != bson_value::view{b_int64{200}});
368368
}
369+
370+
TEST_CASE("document uninitialized element throws exceptions", "") {
371+
using bsoncxx::builder::basic::kvp;
372+
using bsoncxx::builder::basic::make_document;
373+
bsoncxx::document::value doc = make_document(kvp("foo", "bar"));
374+
375+
REQUIRE_THROWS_WITH(doc["doesnotexist"].get_string().value,
376+
Catch::Contains("cannot get string from an uninitialized element with key "
377+
"\"doesnotexist\": unset document::element"));
378+
379+
REQUIRE_THROWS_WITH(doc["alsodoesnotexist"].get_value(),
380+
Catch::Contains("cannot return the type of uninitialized element with key "
381+
"\"alsodoesnotexist\": unset document::element"));
382+
383+
// Ensure a non-existing element evaluates to false.
384+
REQUIRE(!doc["doesnotexist"]);
385+
// Ensure finding a non-existing element results in an end iterator.
386+
REQUIRE(doc.find("doesnotexist") == doc.cend());
387+
// Ensure getting a key from a non-existing element results in an exception.
388+
REQUIRE_THROWS_WITH(
389+
doc["doesnotexist"].key(),
390+
Catch::Contains("cannot return the key from an uninitialized element with key "
391+
"\"doesnotexist\": unset document::element"));
392+
}
393+
394+
TEST_CASE("array uninitialized element throws exceptions", "") {
395+
using bsoncxx::builder::basic::kvp;
396+
using bsoncxx::builder::basic::make_array;
397+
bsoncxx::array::value arr = make_array("a", "b", "c");
398+
399+
REQUIRE_THROWS_WITH(arr.view()[3].get_string().value,
400+
Catch::Contains("cannot get string from an uninitialized element with key "
401+
"\"3\": unset document::element"));
402+
// Ensure a non-existing element evaluates to false.
403+
REQUIRE(!arr.view()[3]);
404+
// Ensure finding a non-existing element results in an end iterator.
405+
REQUIRE(arr.view().find(3) == arr.view().cend());
406+
// Ensure getting a key from a non-existing element results in an exception.
407+
REQUIRE_THROWS_WITH(
408+
arr.view()[3].key(),
409+
Catch::Contains("cannot return the key from an uninitialized element with key "
410+
"\"3\": unset document::element"));
411+
}
369412
} // namespace

0 commit comments

Comments
 (0)