Add non-throwing version of unpackProperties
Some projects (e.g., bmcweb) don't use functions which throw exceptions.
This change introduces unpackPropertiesNoThrow() function, which returns
optional string. In case of mismatched type or missing property, it will
contain its name. In case of no errors, std::nullopt will be returned.
As a side change, message returned by UnpackPropertyError::what() now
also includes propertyName and reason.
Testing done:
- Added unit tests for unpackPropertiesNoThrow().
- unpackProperties() functionality remained the same.
Signed-off-by: Szymon Dompke <szymon.dompke@intel.com>
Change-Id: I61318bb906de7d5a252414c1d3ea25322874e23e
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
diff --git a/include/sdbusplus/exception.hpp b/include/sdbusplus/exception.hpp
index b02e993..516f6f3 100644
--- a/include/sdbusplus/exception.hpp
+++ b/include/sdbusplus/exception.hpp
@@ -121,6 +121,9 @@
const std::string propertyName;
const std::string reason;
+
+ private:
+ const std::string errWhatDetailed;
};
} // namespace exception
diff --git a/include/sdbusplus/unpack_properties.hpp b/include/sdbusplus/unpack_properties.hpp
index c204337..4c1adfa 100644
--- a/include/sdbusplus/unpack_properties.hpp
+++ b/include/sdbusplus/unpack_properties.hpp
@@ -17,7 +17,7 @@
{
template <typename Variant, typename ValueType>
-bool getIf(Variant&& variant, ValueType& outValue)
+bool getIf(Variant&& variant, ValueType& outValue) noexcept
{
if (auto value = std::get_if<ValueType>(&variant))
{
@@ -29,7 +29,7 @@
}
template <typename Container>
-auto findProperty(Container&& container, const std::string& key)
+auto findProperty(Container&& container, const std::string& key) noexcept
{
if constexpr (utility::has_member_find_v<Container>)
{
@@ -44,7 +44,7 @@
}
template <typename Container>
-bool containsProperty(Container&& container, const std::string& key)
+bool containsProperty(Container&& container, const std::string& key) noexcept
{
if constexpr (utility::has_member_contains_v<Container>)
{
@@ -61,7 +61,7 @@
typename... Args>
void readProperties(Container&& container, std::bitset<N>& assigned,
const std::string& expectedKey, ValueType& outValue,
- Args&&... args)
+ Args&&... args) noexcept
{
static_assert(Index < N);
@@ -85,7 +85,7 @@
template <size_t Index, size_t N, typename ValueType, typename... Args>
std::string findMissingProperty(std::bitset<N>& assigned,
const std::string& key, ValueType&,
- Args&&... args)
+ Args&&... args) noexcept
{
static_assert(Index < N);
@@ -103,10 +103,9 @@
return {};
}
-} // namespace detail
-
-template <typename Container, typename... Args>
-void unpackProperties(Container&& input, Args&&... args)
+template <bool ReturnBadProperty, typename Container, typename... Args>
+auto unpackPropertiesCommon(Container&& input,
+ Args&&... args) noexcept(ReturnBadProperty)
{
static_assert(sizeof...(Args) % 2 == 0);
@@ -116,23 +115,49 @@
if (!assigned.all())
{
- std::string missingProperty = detail::findMissingProperty<0>(
+ auto missingProperty = detail::findMissingProperty<0>(
assigned, std::forward<Args>(args)...);
- if (detail::containsProperty(std::forward<Container>(input),
- missingProperty))
+ if constexpr (ReturnBadProperty)
{
- throw exception::UnpackPropertyError(
- missingProperty,
- exception::UnpackPropertyError::reasonTypeNotMatched);
+ return std::optional{missingProperty};
}
else
{
- throw exception::UnpackPropertyError(
- missingProperty,
- exception::UnpackPropertyError::reasonMissingProperty);
+ if (detail::containsProperty(std::forward<Container>(input),
+ missingProperty))
+ {
+ throw exception::UnpackPropertyError(
+ missingProperty,
+ exception::UnpackPropertyError::reasonTypeNotMatched);
+ }
+ else
+ {
+ throw exception::UnpackPropertyError(
+ missingProperty,
+ exception::UnpackPropertyError::reasonMissingProperty);
+ }
}
}
+ return std::conditional_t<ReturnBadProperty, std::optional<std::string>,
+ void>();
+}
+
+} // namespace detail
+
+template <typename Container, typename... Args>
+void unpackProperties(Container&& input, Args&&... args)
+{
+ detail::unpackPropertiesCommon<false, Container, Args...>(
+ std::forward<Container>(input), std::forward<Args>(args)...);
+}
+
+template <typename Container, typename... Args>
+std::optional<std::string> unpackPropertiesNoThrow(Container&& input,
+ Args&&... args) noexcept
+{
+ return detail::unpackPropertiesCommon<true, Container, Args...>(
+ std::forward<Container>(input), std::forward<Args>(args)...);
}
} // namespace sdbusplus
diff --git a/src/exception.cpp b/src/exception.cpp
index f44cdaa..4a04dea 100644
--- a/src/exception.cpp
+++ b/src/exception.cpp
@@ -139,10 +139,12 @@
return EINVAL;
}
-UnpackPropertyError::UnpackPropertyError(std::string_view propertyName,
- std::string_view reason) :
- propertyName(propertyName),
- reason(reason)
+UnpackPropertyError::UnpackPropertyError(std::string_view propertyNameIn,
+ std::string_view reasonIn) :
+ propertyName(propertyNameIn),
+ reason(reasonIn),
+ errWhatDetailed(std::string(errWhat) + " PropertyName: '" + propertyName +
+ "', Reason: '" + reason + "'.")
{}
const char* UnpackPropertyError::name() const noexcept
@@ -157,7 +159,7 @@
const char* UnpackPropertyError::what() const noexcept
{
- return errWhat;
+ return errWhatDetailed.c_str();
}
int UnpackPropertyError::get_errno() const noexcept
diff --git a/test/unpack_properties.cpp b/test/unpack_properties.cpp
index 90415cb..cb7b122 100644
--- a/test/unpack_properties.cpp
+++ b/test/unpack_properties.cpp
@@ -6,11 +6,44 @@
namespace sdbusplus
{
+struct ThrowingUnpack
+{
+ template <typename... Args>
+ bool operator()(Args&&... args) const
+ {
+ unpackProperties(std::forward<Args>(args)...);
+ return false;
+ }
+};
+
+struct NonThrowingUnpack
+{
+ template <typename... Args>
+ std::optional<std::string> operator()(Args&&... args) const
+ {
+ return unpackPropertiesNoThrow(std::forward<Args>(args)...);
+ }
+};
+
+template <typename A, typename B>
+struct TestingTypes
+{
+ using SystemUnderTest = A;
+ using Container = B;
+};
+
using VariantType = std::variant<std::string, uint32_t, float, double>;
-using ContainerTypes =
- testing::Types<std::vector<std::pair<std::string, VariantType>>,
- boost::container::flat_map<std::string, VariantType>,
- std::map<std::string, VariantType>>;
+using ContainerTypes = testing::Types<
+ TestingTypes<NonThrowingUnpack,
+ std::vector<std::pair<std::string, VariantType>>>,
+ TestingTypes<NonThrowingUnpack,
+ boost::container::flat_map<std::string, VariantType>>,
+ TestingTypes<NonThrowingUnpack, std::map<std::string, VariantType>>,
+ TestingTypes<ThrowingUnpack,
+ std::vector<std::pair<std::string, VariantType>>>,
+ TestingTypes<ThrowingUnpack,
+ boost::container::flat_map<std::string, VariantType>>,
+ TestingTypes<ThrowingUnpack, std::map<std::string, VariantType>>>;
template <typename Exception, typename F>
std::optional<Exception> captureException(F&& code)
@@ -27,7 +60,7 @@
return std::nullopt;
}
-template <typename Container>
+template <typename Params>
struct UnpackPropertiesTest : public testing::Test
{
void SetUp() override
@@ -40,7 +73,8 @@
data.insert(data.end(), std::make_pair("Key-3"s, VariantType(15.)));
}
- Container data;
+ typename Params::Container data;
+ typename Params::SystemUnderTest unpackPropertiesCall;
};
TYPED_TEST_SUITE(UnpackPropertiesTest, ContainerTypes);
@@ -53,7 +87,8 @@
float val2 = 0.f;
double val3 = 0.;
- unpackProperties(this->data, "Key-1", val1, "Key-2", val2, "Key-3", val3);
+ EXPECT_FALSE(this->unpackPropertiesCall(this->data, "Key-1", val1, "Key-2",
+ val2, "Key-3", val3));
ASSERT_THAT(val1, Eq("string"));
ASSERT_THAT(val2, FloatEq(42.f));
@@ -67,8 +102,8 @@
std::string val1, val2;
- unpackProperties(this->data, "Key-1", val1);
- unpackProperties(this->data, "Key-1", val2);
+ EXPECT_FALSE(this->unpackPropertiesCall(this->data, "Key-1", val1));
+ EXPECT_FALSE(this->unpackPropertiesCall(this->data, "Key-1", val2));
ASSERT_THAT(val1, Eq("string"));
ASSERT_THAT(val2, Not(Eq("string")));
@@ -81,51 +116,13 @@
std::string val1, val2;
- unpackProperties(Const(this->data), "Key-1", val1);
- unpackProperties(Const(this->data), "Key-1", val2);
+ EXPECT_FALSE(this->unpackPropertiesCall(Const(this->data), "Key-1", val1));
+ EXPECT_FALSE(this->unpackPropertiesCall(Const(this->data), "Key-1", val2));
ASSERT_THAT(val1, Eq("string"));
ASSERT_THAT(val2, Eq("string"));
}
-TYPED_TEST(UnpackPropertiesTest, throwsErrorWhenKeyIsMissing)
-{
- using namespace testing;
-
- std::string val1;
- float val2 = 0.f;
- double val3 = 0.;
-
- auto error = captureException<exception::UnpackPropertyError>([&] {
- unpackProperties(this->data, "Key-1", val1, "Key-4", val2, "Key-3",
- val3);
- });
-
- ASSERT_TRUE(error);
- ASSERT_THAT(error->reason,
- Eq(exception::UnpackPropertyError::reasonMissingProperty));
- ASSERT_THAT(error->propertyName, Eq("Key-4"));
-}
-
-TYPED_TEST(UnpackPropertiesTest, throwsErrorWhenTypeDoesntMatch)
-{
- using namespace testing;
-
- std::string val1;
- std::string val2;
- double val3 = 0.;
-
- auto error = captureException<exception::UnpackPropertyError>([&] {
- unpackProperties(this->data, "Key-1", val1, "Key-2", val2, "Key-3",
- val3);
- });
-
- ASSERT_TRUE(error);
- ASSERT_THAT(error->reason,
- Eq(exception::UnpackPropertyError::reasonTypeNotMatched));
- ASSERT_THAT(error->propertyName, Eq("Key-2"));
-}
-
TYPED_TEST(UnpackPropertiesTest,
returnsUndefinedValueForDuplicatedKeysWhenDataIsNonConstReference)
{
@@ -137,8 +134,9 @@
double val3 = 0.;
std::string val4;
- unpackProperties(this->data, "Key-1", val1, "Key-2", val2, "Key-3", val3,
- "Key-1", val4);
+ EXPECT_FALSE(this->unpackPropertiesCall(this->data, "Key-1", val1, "Key-2",
+ val2, "Key-3", val3, "Key-1",
+ val4));
ASSERT_THAT(val1, Eq("string"));
ASSERT_THAT(val2, FloatEq(42.f));
@@ -157,8 +155,9 @@
double val3 = 0.;
std::string val4;
- unpackProperties(Const(this->data), "Key-1", val1, "Key-2", val2, "Key-3",
- val3, "Key-1", val4);
+ EXPECT_FALSE(this->unpackPropertiesCall(Const(this->data), "Key-1", val1,
+ "Key-2", val2, "Key-3", val3,
+ "Key-1", val4));
ASSERT_THAT(val1, Eq("string"));
ASSERT_THAT(val2, FloatEq(42.f));
@@ -166,12 +165,113 @@
ASSERT_THAT(val4, Eq("string"));
}
-struct UnpackPropertiesTest_ForVector :
- public UnpackPropertiesTest<
- std::vector<std::pair<std::string, VariantType>>>
+template <typename Params>
+struct UnpackPropertiesThrowingTest : public UnpackPropertiesTest<Params>
{};
-TEST_F(UnpackPropertiesTest_ForVector, silentlyDiscardsDuplicatedKeyInData)
+using ContainerTypesThrowing = testing::Types<
+ TestingTypes<ThrowingUnpack,
+ std::vector<std::pair<std::string, VariantType>>>,
+ TestingTypes<ThrowingUnpack,
+ boost::container::flat_map<std::string, VariantType>>,
+ TestingTypes<ThrowingUnpack, std::map<std::string, VariantType>>>;
+
+TYPED_TEST_SUITE(UnpackPropertiesThrowingTest, ContainerTypesThrowing);
+
+TYPED_TEST(UnpackPropertiesThrowingTest, throwsErrorWhenKeyIsMissing)
+{
+ using namespace testing;
+
+ std::string val1;
+ float val2 = 0.f;
+ double val3 = 0.;
+
+ auto error = captureException<exception::UnpackPropertyError>([&] {
+ this->unpackPropertiesCall(this->data, "Key-1", val1, "Key-4", val2,
+ "Key-3", val3);
+ });
+
+ ASSERT_TRUE(error);
+ ASSERT_THAT(error->reason,
+ Eq(exception::UnpackPropertyError::reasonMissingProperty));
+ ASSERT_THAT(error->propertyName, Eq("Key-4"));
+}
+
+TYPED_TEST(UnpackPropertiesThrowingTest, throwsErrorWhenTypeDoesntMatch)
+{
+ using namespace testing;
+
+ std::string val1;
+ std::string val2;
+ double val3 = 0.;
+
+ auto error = captureException<exception::UnpackPropertyError>([&] {
+ this->unpackPropertiesCall(this->data, "Key-1", val1, "Key-2", val2,
+ "Key-3", val3);
+ });
+
+ ASSERT_TRUE(error);
+ ASSERT_THAT(error->reason,
+ Eq(exception::UnpackPropertyError::reasonTypeNotMatched));
+ ASSERT_THAT(error->propertyName, Eq("Key-2"));
+}
+
+template <typename Params>
+struct UnpackPropertiesNonThrowingTest : public UnpackPropertiesTest<Params>
+{};
+
+using ContainerTypesNonThrowing = testing::Types<
+ TestingTypes<NonThrowingUnpack,
+ std::vector<std::pair<std::string, VariantType>>>,
+ TestingTypes<NonThrowingUnpack,
+ boost::container::flat_map<std::string, VariantType>>,
+ TestingTypes<NonThrowingUnpack, std::map<std::string, VariantType>>>;
+
+TYPED_TEST_SUITE(UnpackPropertiesNonThrowingTest, ContainerTypesNonThrowing);
+
+TYPED_TEST(UnpackPropertiesNonThrowingTest, ErrorWhenKeyIsMissing)
+{
+ using namespace testing;
+
+ std::string val1;
+ float val2 = 0.f;
+ double val3 = 0.;
+
+ auto badProperty = this->unpackPropertiesCall(this->data, "Key-1", val1,
+ "Key-4", val2, "Key-3", val3);
+
+ ASSERT_TRUE(badProperty);
+ ASSERT_THAT(*badProperty, Eq("Key-4"));
+}
+
+TYPED_TEST(UnpackPropertiesNonThrowingTest, ErrorWhenTypeDoesntMatch)
+{
+ using namespace testing;
+
+ std::string val1;
+ std::string val2;
+ double val3 = 0.;
+
+ auto badProperty = this->unpackPropertiesCall(this->data, "Key-1", val1,
+ "Key-2", val2, "Key-3", val3);
+
+ ASSERT_TRUE(badProperty);
+ ASSERT_THAT(*badProperty, Eq("Key-2"));
+}
+
+template <typename Params>
+struct UnpackPropertiesTest_ForVector : public UnpackPropertiesTest<Params>
+{};
+
+using ContainerTypesVector = testing::Types<
+ TestingTypes<NonThrowingUnpack,
+ std::vector<std::pair<std::string, VariantType>>>,
+ TestingTypes<ThrowingUnpack,
+ std::vector<std::pair<std::string, VariantType>>>>;
+
+TYPED_TEST_SUITE(UnpackPropertiesTest_ForVector, ContainerTypesVector);
+
+TYPED_TEST(UnpackPropertiesTest_ForVector, silentlyDiscardsDuplicatedKeyInData)
{
using namespace testing;
using namespace std::string_literals;
@@ -183,7 +283,8 @@
this->data.insert(this->data.end(),
std::make_pair("Key-1"s, VariantType("string2"s)));
- unpackProperties(this->data, "Key-1", val1, "Key-2", val2, "Key-3", val3);
+ EXPECT_FALSE(this->unpackPropertiesCall(this->data, "Key-1", val1, "Key-2",
+ val2, "Key-3", val3));
ASSERT_THAT(val1, Eq("string"));
ASSERT_THAT(val2, FloatEq(42.f));