dbus: passive: move exception checking into factory

Move the exception checking into the factory to remove possibility of
exception during construction.

Change-Id: I22922f23247c0579ee2a3411b5c8cede44fe324c
Signed-off-by: Patrick Venture <venture@google.com>

4 files changed, 35 insertions, 32 deletions

diff --git a/dbus/dbuspassive.cpp b/dbus/dbuspassive.cpp
index 7e86790..3a6d6d4 100644
--- a/dbus/dbuspassive.cpp
+++ b/dbus/dbuspassive.cpp
@@ -37,30 +37,41 @@ std::unique_ptr<ReadInterface> DbusPassive::createDbusPassive(
         return nullptr;
     }
 
-    return std::make_unique<DbusPassive>(bus, type, id, helper);
-}
-
-DbusPassive::DbusPassive(sdbusplus::bus::bus& bus, const std::string& type,
-                         const std::string& id, DbusHelperInterface* helper) :
-    ReadInterface(),
-    _bus(bus), _signal(bus, getMatch(type, id).c_str(), dbusHandleSignal, this),
-    _id(id), _helper(helper)
-{
     /* Need to get the scale and initial value */
     auto tempBus = sdbusplus::bus::new_default();
+
     /* service == busname */
     std::string path = getSensorPath(type, id);
 
-    /* getService can except, should this be in the factory? */
-    std::string service = _helper->getService(tempBus, sensorintf, path);
-
     struct SensorProperties settings;
-    _helper->getProperties(tempBus, service, path, &settings);
+    bool failed;
 
+    try
+    {
+        std::string service = helper->getService(tempBus, sensorintf, path);
+
+        helper->getProperties(tempBus, service, path, &settings);
+        failed = helper->thresholdsAsserted(tempBus, service, path);
+    }
+    catch (const std::exception& e)
+    {
+        return nullptr;
+    }
+
+    return std::make_unique<DbusPassive>(bus, type, id, helper, settings,
+                                         failed);
+}
+
+DbusPassive::DbusPassive(sdbusplus::bus::bus& bus, const std::string& type,
+                         const std::string& id, DbusHelperInterface* helper,
+                         const struct SensorProperties& settings, bool failed) :
+    ReadInterface(),
+    _bus(bus), _signal(bus, getMatch(type, id).c_str(), dbusHandleSignal, this),
+    _id(id), _helper(helper), _failed(failed)
+{
     _scale = settings.scale;
     _value = settings.value * pow(10, _scale);
     _updated = std::chrono::high_resolution_clock::now();
-    _failed = _helper->thresholdsAsserted(tempBus, service, path);
 }
 
 ReadReturn DbusPassive::read(void)
diff --git a/dbus/dbuspassive.hpp b/dbus/dbuspassive.hpp
index 02e395c..5dbd3c3 100644
--- a/dbus/dbuspassive.hpp
+++ b/dbus/dbuspassive.hpp
@@ -38,7 +38,8 @@ class DbusPassive : public ReadInterface
                           const std::string& id, DbusHelperInterface* helper);
 
     DbusPassive(sdbusplus::bus::bus& bus, const std::string& type,
-                const std::string& id, DbusHelperInterface* helper);
+                const std::string& id, DbusHelperInterface* helper,
+                const struct SensorProperties& settings, bool failed);
 
     ReadReturn read(void) override;
     bool getFailed(void) const override;
diff --git a/dbus/util.hpp b/dbus/util.hpp
index 67125b9..1b3a95b 100644
--- a/dbus/util.hpp
+++ b/dbus/util.hpp
@@ -27,6 +27,8 @@ class DbusHelperInterface
     virtual ~DbusHelperInterface() = default;
 
     /** @brief Get the service providing the interface for the path.
+     *
+     * @warning Throws exception on dbus failure.
      */
     virtual std::string getService(sdbusplus::bus::bus& bus,
                                    const std::string& intf,
@@ -38,6 +40,8 @@ class DbusHelperInterface
      * @param[in] service - The service providing the interface.
      * @param[in] path - The dbus path.
      * @param[out] prop - A pointer to a properties struct to fill out.
+     *
+     * @warning Throws exception on dbus failure.
      */
     virtual void getProperties(sdbusplus::bus::bus& bus,
                                const std::string& service,
diff --git a/test/dbus_passive_unittest.cpp b/test/dbus_passive_unittest.cpp
index 6826ce1..52126cf 100644
--- a/test/dbus_passive_unittest.cpp
+++ b/test/dbus_passive_unittest.cpp
@@ -36,8 +36,7 @@ TEST(DbusPassiveTest, FactoryFailsWithInvalidType)
 
 TEST(DbusPassiveTest, BoringConstructorTest)
 {
-    // Just build the object, which should be avoided as this does no error
-    // checking at present.
+    // Simply build the object, does no error checking.
 
     sdbusplus::SdBusMock sdbus_mock;
     auto bus_mock = sdbusplus::get_mocked_new(&sdbus_mock);
@@ -46,21 +45,9 @@ TEST(DbusPassiveTest, BoringConstructorTest)
     std::string path = "/xyz/openbmc_project/sensors/unknown/id";
 
     DbusHelperMock helper;
-    EXPECT_CALL(helper, getService(_, StrEq(SensorIntf), StrEq(path)))
-        .WillOnce(Return("asdf"));
-
-    EXPECT_CALL(helper, getProperties(_, StrEq("asdf"), StrEq(path), NotNull()))
-        .WillOnce(
-            Invoke([&](sdbusplus::bus::bus& bus, const std::string& service,
-                       const std::string& path, struct SensorProperties* prop) {
-                prop->scale = -3;
-                prop->value = 10;
-                prop->unit = "x";
-            }));
-    EXPECT_CALL(helper, thresholdsAsserted(_, StrEq("asdf"), StrEq(path)))
-        .WillOnce(Return(false));
-
-    DbusPassive(bus_mock, type, id, &helper);
+    struct SensorProperties properties;
+
+    DbusPassive(bus_mock, type, id, &helper, properties, false);
     // Success
 }