Save properties to persistent storage when host is on

1. When host is on, set properties as requested properties instead
of notify listeners;
2. When host becomes off, and requested properties are not empty, notify
the listners and reset the requested properties.

Add unit tests.

Change-Id: I9359c801c698df0c6e5eab43e12427bb5a6da611
Signed-off-by: Lei YU <mine260309@gmail.com>

7 files changed, 263 insertions, 85 deletions

diff --git a/host_epoch.cpp b/host_epoch.cpp
index f45581d..3b4c00e 100644
--- a/host_epoch.cpp
+++ b/host_epoch.cpp
@@ -1,9 +1,8 @@
 #include "host_epoch.hpp"
+#include "utils.hpp"
 
 #include <phosphor-logging/log.hpp>
 
-#include <fstream>
-
 namespace phosphor
 {
 namespace time
@@ -14,7 +13,7 @@ using namespace phosphor::logging;
 HostEpoch::HostEpoch(sdbusplus::bus::bus& bus,
                      const char* objPath)
     : EpochBase(bus, objPath),
-      offset(readData<decltype(offset)::rep>(offsetFile))
+      offset(utils::readData<decltype(offset)::rep>(offsetFile))
 {
     // Empty
 }
@@ -41,34 +40,12 @@ uint64_t HostEpoch::elapsed(uint64_t value)
     offset = time - getTime();
 
     // Store the offset to file
-    writeData(offsetFile, offset.count());
+    utils::writeData(offsetFile, offset.count());
 
     server::EpochTime::elapsed(value);
     return value;
 }
 
-template <typename T>
-T HostEpoch::readData(const char* fileName)
-{
-    T data{};
-    std::ifstream fs(fileName);
-    if (fs.is_open())
-    {
-        fs >> data;
-    }
-    return data;
-}
-
-template <typename T>
-void HostEpoch::writeData(const char* fileName, T&& data)
-{
-    std::ofstream fs(fileName, std::ios::out);
-    if (fs.is_open())
-    {
-        fs << std::forward<T>(data);
-    }
-}
-
 } // namespace time
 } // namespace phosphor
 
diff --git a/host_epoch.hpp b/host_epoch.hpp
index 1252ff9..3798f8f 100644
--- a/host_epoch.hpp
+++ b/host_epoch.hpp
@@ -46,23 +46,6 @@ class HostEpoch : public EpochBase
          *  Read back when starts
          **/
         static constexpr auto offsetFile = HOST_OFFSET_FILE;
-
-        /** @brief Read data with type T from file
-         *
-         * @param[in] fileName - The name of file to read from
-         *
-         * @return The data with type T
-         */
-        template <typename T>
-        static T readData(const char* fileName);
-
-        /** @brief Write data with type T to file
-         *
-         * @param[in] fileName - The name of file to write to
-         * @param[in] data - The data with type T to write to file
-         */
-        template <typename T>
-        static void writeData(const char* fileName, T&& data);
 };
 
 } // namespace time
diff --git a/manager.cpp b/manager.cpp
index 5a3d928..9810e14 100644
--- a/manager.cpp
+++ b/manager.cpp
@@ -1,4 +1,5 @@
 #include "manager.hpp"
+#include "utils.hpp"
 
 #include <phosphor-logging/log.hpp>
 
@@ -12,9 +13,6 @@ constexpr auto SETTINGS_INTERFACE = "org.openbmc.settings.Host";
 constexpr auto PROPERTY_INTERFACE = "org.freedesktop.DBus.Properties";
 constexpr auto METHOD_GET = "Get";
 
-constexpr auto PROPERTY_TIME_MODE = "time_mode";
-constexpr auto PROPERTY_TIME_OWNER = "time_owner";
-
 // TODO: Use new settings in xyz.openbmc_project
 const auto MATCH_PROPERTY_CHANGE =
     rules::type::signal() +
@@ -58,9 +56,14 @@ Manager::Manager(sdbusplus::bus::bus& bus)
       propertyChangeMatch(bus, MATCH_PROPERTY_CHANGE, onPropertyChanged, this),
       pgoodChangeMatch(bus, MATCH_PGOOD_CHANGE, onPgoodChanged, this)
 {
-    setCurrentTimeMode(getSettings(PROPERTY_TIME_MODE));
-    setCurrentTimeOwner(getSettings(PROPERTY_TIME_OWNER));
     checkHostOn();
+
+    // Restore settings from persistent storage
+    restoreSettings();
+
+    // Check the settings daemon to process the new settings
+    onPropertyChanged(PROPERTY_TIME_MODE, getSettings(PROPERTY_TIME_MODE));
+    onPropertyChanged(PROPERTY_TIME_OWNER, getSettings(PROPERTY_TIME_OWNER));
 }
 
 void Manager::addListener(PropertyChangeListner* listener)
@@ -72,6 +75,20 @@ void Manager::addListener(PropertyChangeListner* listener)
     listeners.insert(listener);
 }
 
+void Manager::restoreSettings()
+{
+    auto mode = utils::readData<std::string>(modeFile);
+    if (!mode.empty())
+    {
+        timeMode = convertToMode(mode);
+    }
+    auto owner = utils::readData<std::string>(ownerFile);
+    if (!owner.empty())
+    {
+        timeOwner = convertToOwner(owner);
+    }
+}
+
 void Manager::checkHostOn()
 {
     sdbusplus::message::variant<int> pgood = 0;
@@ -92,28 +109,32 @@ void Manager::checkHostOn()
 void Manager::onPropertyChanged(const std::string& key,
                                 const std::string& value)
 {
-    // TODO: Check pgood
-    // If it's off, notify listners;
-    // If it's on, hold the values and store in persistent storage
-    // as requested time mode/owner.
-    // And when pgood turns back to off, notify the listners.
-
     // TODO: Check dhcp_ntp
 
-    if (key == PROPERTY_TIME_MODE)
+    if (hostOn)
     {
-        setCurrentTimeMode(value);
-        for (const auto& listener : listeners)
-        {
-            listener->onModeChanged(timeMode);
-        }
+        // If host is on, set the values as requested time mode/owner.
+        // And when host becomes off, notify the listners.
+        setPropertyAsRequested(key, value);
     }
-    else if (key == PROPERTY_TIME_OWNER)
+    else
     {
-        setCurrentTimeOwner(value);
-        for (const auto& listener : listeners)
+        // If host is off, notify listners
+        if (key == PROPERTY_TIME_MODE)
         {
-            listener->onOwnerChanged(timeOwner);
+            setCurrentTimeMode(value);
+            for (const auto listener : listeners)
+            {
+                listener->onModeChanged(timeMode);
+            }
+        }
+        else if (key == PROPERTY_TIME_OWNER)
+        {
+            setCurrentTimeOwner(value);
+            for (const auto listener : listeners)
+            {
+                listener->onOwnerChanged(timeOwner);
+            }
         }
     }
 }
@@ -133,18 +154,67 @@ int Manager::onPropertyChanged(sd_bus_message* msg,
     {
         if (managedProperties.find(item.first) != managedProperties.end())
         {
-            static_cast<Manager*>(userData)
-                ->onPropertyChanged(item.first, item.second.get<std::string>());
+            static_cast<Manager*>(userData)->onPropertyChanged(
+                item.first, item.second.get<std::string>());
         }
     }
     return 0;
 }
 
+void Manager::setPropertyAsRequested(const std::string& key,
+                                     const std::string& value)
+{
+    if (key == PROPERTY_TIME_MODE)
+    {
+        setRequestedMode(value);
+    }
+    else if (key == PROPERTY_TIME_OWNER)
+    {
+        setRequestedOwner(value);
+    }
+    else
+    {
+        // The key shall be already the supported one
+        // TODO: use elog API
+        assert(false);
+    }
+}
+
+void Manager::setRequestedMode(const std::string& mode)
+{
+    requestedMode = mode;
+}
+
+void Manager::setRequestedOwner(const std::string& owner)
+{
+    requestedOwner = owner;
+}
+
 void Manager::onPgoodChanged(bool pgood)
 {
     hostOn = pgood;
-    // TODO: if host is off, check requested time_mode/owner:
-    // and notify the listeners if any.
+    if (hostOn)
+    {
+        return;
+    }
+    if (!requestedMode.empty())
+    {
+        setCurrentTimeMode(requestedMode);
+        for (const auto& listener : listeners)
+        {
+            listener->onModeChanged(timeMode);
+        }
+        setRequestedMode({}); // Clear requested mode
+    }
+    if (!requestedOwner.empty())
+    {
+        setCurrentTimeOwner(requestedOwner);
+        for (const auto& listener : listeners)
+        {
+            listener->onOwnerChanged(timeOwner);
+        }
+        setRequestedOwner({}); // Clear requested owner
+    }
 }
 
 int Manager::onPgoodChanged(sd_bus_message* msg,
@@ -174,6 +244,7 @@ void Manager::setCurrentTimeMode(const std::string& mode)
     log<level::INFO>("Time mode is changed",
                      entry("MODE=%s", mode.c_str()));
     timeMode = convertToMode(mode);
+    utils::writeData(modeFile, mode);
 }
 
 void Manager::setCurrentTimeOwner(const std::string& owner)
@@ -181,6 +252,7 @@ void Manager::setCurrentTimeOwner(const std::string& owner)
     log<level::INFO>("Time owner is changed",
                      entry("OWNER=%s", owner.c_str()));
     timeOwner = convertToOwner(owner);
+    utils::writeData(ownerFile, owner);
 }
 
 std::string Manager::getSettings(const char* value) const
diff --git a/manager.hpp b/manager.hpp
index ef3315f..7edb30f 100644
--- a/manager.hpp
+++ b/manager.hpp
@@ -24,6 +24,7 @@ class Manager
 {
     public:
         friend class TestManager;
+
         explicit Manager(sdbusplus::bus::bus& bus);
         Manager(const Manager&) = delete;
         Manager& operator=(const Manager&) = delete;
@@ -51,12 +52,21 @@ class Manager
         /** @brief The value to indicate if host is on */
         bool hostOn = false;
 
+        /** @brief The requested time mode when host is on*/
+        std::string requestedMode;
+
+        /** @brief The requested time owner when host is on*/
+        std::string requestedOwner;
+
         /** @brief The current time mode */
         Mode timeMode;
 
         /** @brief The current time owner */
         Owner timeOwner;
 
+        /** @brief Restore saved settings */
+        void restoreSettings();
+
         /** @brief Check if host is on and update hostOn variable */
         void checkHostOn();
 
@@ -94,6 +104,28 @@ class Manager
          */
         void onPgoodChanged(bool pgood);
 
+        /** @brief Set the property as requested time mode/owner
+         *
+         * @param[in] key - The property name
+         * @param[in] value - The property value
+         */
+        void setPropertyAsRequested(const std::string& key,
+                                    const std::string& value);
+
+        /** @brief Set the current mode to user requested one
+         *  if conditions allow it
+         *
+         * @param[in] mode - The string of time mode
+         */
+        void setRequestedMode(const std::string& mode);
+
+        /** @brief Set the current owner to user requested one
+         *  if conditions allow it
+         *
+         * @param[in] owner - The string of time owner
+         */
+        void setRequestedOwner(const std::string& owner);
+
         /** @brief The static function called on settings property changed
          *
          * @param[in] msg - Data associated with subscribed signal
@@ -138,6 +170,12 @@ class Manager
          */
         static Owner convertToOwner(const std::string& owner);
 
+        /** @brief The string of time mode property */
+        static constexpr auto PROPERTY_TIME_MODE = "time_mode";
+
+        /** @brief The string of time owner property */
+        static constexpr auto PROPERTY_TIME_OWNER = "time_owner";
+
         using Updater = std::function<void(const std::string&)>;
 
         /** @brief Map the property string to functions that shall
@@ -145,10 +183,10 @@ class Manager
          */
         const std::map<std::string, Updater> propertyUpdaters =
         {
-            {"time_mode", std::bind(&Manager::setCurrentTimeMode,
-                                    this, std::placeholders::_1)},
-            {"time_owner", std::bind(&Manager::setCurrentTimeOwner,
-                                     this, std::placeholders::_1)}
+            {PROPERTY_TIME_MODE, std::bind(&Manager::setCurrentTimeMode,
+                                           this, std::placeholders::_1)},
+            {PROPERTY_TIME_OWNER, std::bind(&Manager::setCurrentTimeOwner,
+                                            this, std::placeholders::_1)}
         };
 
         /** @brief The properties that manager shall notify the
@@ -158,6 +196,12 @@ class Manager
 
         /** @brief The map that maps the string to Owners */
         static const std::map<std::string, Owner> ownerMap;
+
+        /** @brief The file name of saved time mode */
+        static constexpr auto modeFile = "/var/lib/obmc/saved_time_mode";
+
+        /** @brief The file name of saved time owner */
+        static constexpr auto ownerFile = "/var/lib/obmc/saved_time_owner";
 };
 
 }
diff --git a/test/TestHostEpoch.cpp b/test/TestHostEpoch.cpp
index ec8ecf1..0ff8a11 100644
--- a/test/TestHostEpoch.cpp
+++ b/test/TestHostEpoch.cpp
@@ -2,6 +2,7 @@
 #include <gtest/gtest.h>
 
 #include "host_epoch.hpp"
+#include "utils.hpp"
 #include "config.h"
 #include "types.hpp"
 
@@ -45,16 +46,6 @@ class TestHostEpoch : public testing::Test
         {
             return hostEpoch.timeOwner;
         }
-        template <typename T>
-        T readData(const char* fileName)
-        {
-            return HostEpoch::readData<T>(fileName);
-        }
-        template <typename T>
-        void writeData(const char* fileName, T&& data)
-        {
-            HostEpoch::writeData<T>(fileName, std::forward<T>(data));
-        }
         microseconds getOffset()
         {
             return hostEpoch.offset;
@@ -75,7 +66,7 @@ TEST_F(TestHostEpoch, readDataFileNotExist)
 {
     // When file does not exist, the default offset shall be 0
     microseconds offset(0);
-    auto value = readData<decltype(offset)::rep>(FILE_NOT_EXIST);
+    auto value = utils::readData<decltype(offset)::rep>(FILE_NOT_EXIST);
     EXPECT_EQ(0, value);
 }
 
@@ -83,12 +74,13 @@ TEST_F(TestHostEpoch, writeAndReadData)
 {
     // Write offset to file
     microseconds offsetToWrite(1234567);
-    writeData<decltype(offsetToWrite)::rep>(FILE_OFFSET, offsetToWrite.count());
+    utils::writeData<decltype(offsetToWrite)::rep>(
+        FILE_OFFSET, offsetToWrite.count());
 
     // Read it back
     microseconds offsetToRead;
     offsetToRead = microseconds(
-        readData<decltype(offsetToRead)::rep>(FILE_OFFSET));
+        utils::readData<decltype(offsetToRead)::rep>(FILE_OFFSET));
     EXPECT_EQ(offsetToWrite, offsetToRead);
 }
 
diff --git a/test/TestManager.cpp b/test/TestManager.cpp
index 4d9ae73..935590e 100644
--- a/test/TestManager.cpp
+++ b/test/TestManager.cpp
@@ -39,10 +39,34 @@ class TestManager : public testing::Test
         {
             return Manager::convertToOwner(owner);
         }
+        bool hostOn()
+        {
+            return manager.hostOn;
+        }
+        std::string getRequestedMode()
+        {
+            return manager.requestedMode;
+        }
+        std::string getRequestedOwner()
+        {
+            return manager.requestedOwner;
+        }
+        void notifyPropertyChanged(const std::string& key,
+                                   const std::string& value)
+        {
+            manager.onPropertyChanged(key, value);
+        }
+        void notifyPgoodChanged(bool pgood)
+        {
+            manager.onPgoodChanged(pgood);
+        }
 };
 
 TEST_F(TestManager, empty)
 {
+    EXPECT_FALSE(hostOn());
+    EXPECT_EQ("", getRequestedMode());
+    EXPECT_EQ("", getRequestedOwner());
     EXPECT_EQ(Mode::NTP, getTimeMode());
     EXPECT_EQ(Owner::BMC, getTimeOwner());
 }
@@ -72,5 +96,44 @@ TEST_F(TestManager, convertToOwner)
     EXPECT_EQ(Owner::BMC, convertToOwner("xyz"));
 }
 
+TEST_F(TestManager, pgoodChange)
+{
+    notifyPgoodChanged(true);
+    EXPECT_TRUE(hostOn());
+    notifyPgoodChanged(false);
+    EXPECT_FALSE(hostOn());
+}
+
+TEST_F(TestManager, propertyChange)
+{
+    // When host is off, property change will be notified to listners
+    EXPECT_FALSE(hostOn());
+    notifyPropertyChanged("time_mode", "MANUAL");
+    notifyPropertyChanged("time_owner", "HOST");
+    EXPECT_EQ("", getRequestedMode());
+    EXPECT_EQ("", getRequestedOwner());
+    // TODO: if gmock is ready, check mocked listners shall receive notifies
+
+    notifyPgoodChanged(true);
+    // When host is on, property changes are saved as requested ones
+    notifyPropertyChanged("time_mode", "MANUAL");
+    notifyPropertyChanged("time_owner", "HOST");
+    EXPECT_EQ("MANUAL", getRequestedMode());
+    EXPECT_EQ("HOST", getRequestedOwner());
+
+
+    // When host becomes off, the requested mode/owner shall be notified
+    // to listners, and be cleared
+    notifyPgoodChanged(false);
+    // TODO: if gmock is ready, check mocked listners shall receive notifies
+    EXPECT_EQ("", getRequestedMode());
+    EXPECT_EQ("", getRequestedOwner());
+
+    // When host is on, and invalid property is changed,
+    // verify the code asserts because it shall never occur
+    notifyPgoodChanged(true);
+    ASSERT_DEATH(notifyPropertyChanged("invalid property", "whatever"), "");
+}
+
 }
 }
diff --git a/utils.hpp b/utils.hpp
new file mode 100644
index 0000000..0589613
--- /dev/null
+++ b/utils.hpp
@@ -0,0 +1,47 @@
+#pragma once
+
+#include <fstream>
+
+namespace phosphor
+{
+namespace time
+{
+namespace utils
+{
+
+/** @brief Read data with type T from file
+ *
+ * @param[in] fileName - The name of file to read from
+ *
+ * @return The data with type T
+ */
+template <typename T>
+T readData(const char* fileName)
+{
+    T data{};
+    std::ifstream fs(fileName);
+    if (fs.is_open())
+    {
+        fs >> data;
+    }
+    return data;
+}
+
+/** @brief Write data with type T to file
+ *
+ * @param[in] fileName - The name of file to write to
+ * @param[in] data - The data with type T to write to file
+ */
+template <typename T>
+void writeData(const char* fileName, T&& data)
+{
+    std::ofstream fs(fileName, std::ios::out);
+    if (fs.is_open())
+    {
+        fs << std::forward<T>(data);
+    }
+}
+
+}
+}
+}