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/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));