dbus: move zone indexing to util

Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: Ia6560a0e0f976dcf9f05004de68f74f5e313950b
diff --git a/dbus/dbusconfiguration.cpp b/dbus/dbusconfiguration.cpp
index 90c2472..a33a966 100644
--- a/dbus/dbusconfiguration.cpp
+++ b/dbus/dbusconfiguration.cpp
@@ -155,59 +155,6 @@
     std::cout << "}\n\n";
 }
 
-int64_t setZoneIndex(const std::string& name,
-                     std::map<std::string, int64_t>& zones, int64_t index)
-{
-    auto it = zones.find(name);
-    if (it != zones.end())
-    {
-        // Name already allocated, make no change, return existing
-        return it->second;
-    }
-
-    // The zone name is known not to exist yet
-    for (;;)
-    {
-        bool usedIndex = false;
-
-        // See if desired index number is free
-        for (const auto& zi : zones)
-        {
-            if (index == zi.second)
-            {
-                usedIndex = true;
-                break;
-            }
-        }
-
-        // Increment until a free index number is found
-        if (usedIndex)
-        {
-            ++index;
-            continue;
-        }
-
-        break;
-    }
-
-    // Allocate and return new zone index number for this name
-    zones[name] = index;
-    return index;
-}
-
-int64_t getZoneIndex(const std::string& name,
-                     std::map<std::string, int64_t>& zones)
-{
-    auto it = zones.find(name);
-    if (it != zones.end())
-    {
-        return it->second;
-    }
-
-    // Auto-assign next unused zone number, using 0-based numbering
-    return setZoneIndex(name, zones, 0);
-}
-
 std::vector<std::string> getSelectedProfiles(sdbusplus::bus::bus& bus)
 {
     std::vector<std::string> ret;
diff --git a/dbus/dbusutil.cpp b/dbus/dbusutil.cpp
index 6cdad14..4ebe65f 100644
--- a/dbus/dbusutil.cpp
+++ b/dbus/dbusutil.cpp
@@ -19,6 +19,59 @@
 namespace pid_control
 {
 
+int64_t setZoneIndex(const std::string& name,
+                     std::map<std::string, int64_t>& zones, int64_t index)
+{
+    auto it = zones.find(name);
+    if (it != zones.end())
+    {
+        // Name already allocated, make no change, return existing
+        return it->second;
+    }
+
+    // The zone name is known not to exist yet
+    for (;;)
+    {
+        bool usedIndex = false;
+
+        // See if desired index number is free
+        for (const auto& zi : zones)
+        {
+            if (index == zi.second)
+            {
+                usedIndex = true;
+                break;
+            }
+        }
+
+        // Increment until a free index number is found
+        if (usedIndex)
+        {
+            ++index;
+            continue;
+        }
+
+        break;
+    }
+
+    // Allocate and return new zone index number for this name
+    zones[name] = index;
+    return index;
+}
+
+int64_t getZoneIndex(const std::string& name,
+                     std::map<std::string, int64_t>& zones)
+{
+    auto it = zones.find(name);
+    if (it != zones.end())
+    {
+        return it->second;
+    }
+
+    // Auto-assign next unused zone number, using 0-based numbering
+    return setZoneIndex(name, zones, 0);
+}
+
 bool findSensors(const std::unordered_map<std::string, std::string>& sensors,
                  const std::string& search,
                  std::vector<std::pair<std::string, std::string>>& matches)
diff --git a/dbus/dbusutil.hpp b/dbus/dbusutil.hpp
index 420c8fb..7faf4fc 100644
--- a/dbus/dbusutil.hpp
+++ b/dbus/dbusutil.hpp
@@ -1,5 +1,7 @@
 #pragma once
 
+#include <cstdint>
+#include <map>
 #include <stdexcept>
 #include <string>
 #include <unordered_map>
@@ -35,4 +37,12 @@
                  const std::string& search,
                  std::vector<std::pair<std::string, std::string>>& matches);
 
+// Set zone number for a zone, 0-based
+int64_t setZoneIndex(const std::string& name,
+                     std::map<std::string, int64_t>& zones, int64_t index);
+
+// Read zone number for a zone.
+int64_t getZoneIndex(const std::string& name,
+                     std::map<std::string, int64_t>& zones);
+
 } // namespace pid_control
diff --git a/test/dbus_util_unittest.cpp b/test/dbus_util_unittest.cpp
index 0780cb1..a4d2e20 100644
--- a/test/dbus_util_unittest.cpp
+++ b/test/dbus_util_unittest.cpp
@@ -14,6 +14,8 @@
 namespace
 {
 
+using ::testing::ContainerEq;
+using ::testing::Eq;
 using ::testing::StrEq;
 using ::testing::UnorderedElementsAreArray;
 
@@ -85,5 +87,111 @@
     EXPECT_THAT(results, UnorderedElementsAreArray(expected_results));
 }
 
+TEST(GetZoneIndexTest, ZoneAlreadyAssigned)
+{
+    std::map<std::string, int64_t> zones = {
+        {"a", 0},
+    };
+    const std::map<std::string, int64_t> expected_zones = {
+        {"a", 0},
+    };
+
+    EXPECT_THAT(getZoneIndex("a", zones), Eq(0));
+    EXPECT_THAT(zones, ContainerEq(expected_zones));
+}
+
+TEST(GetZoneIndexTest, ZoneNotYetAssignedZeroBased)
+{
+    /* This calls into setZoneIndex, but is a case hit by getZoneIndex. */
+    std::map<std::string, int64_t> zones;
+    const std::map<std::string, int64_t> expected_zones = {
+        {"a", 0},
+    };
+
+    EXPECT_THAT(getZoneIndex("a", zones), Eq(0));
+    EXPECT_THAT(zones, ContainerEq(expected_zones));
+}
+
+TEST(SetZoneIndexTest, ZoneAlreadyAssigned)
+{
+    std::map<std::string, int64_t> zones = {
+        {"a", 0},
+    };
+    const std::map<std::string, int64_t> expected_zones = {
+        {"a", 0},
+    };
+
+    EXPECT_THAT(setZoneIndex("a", zones, 0), Eq(0));
+    EXPECT_THAT(zones, ContainerEq(expected_zones));
+}
+
+TEST(SetZoneIndexTest, ZoneNotYetAssignedEmptyListZeroBased)
+{
+    constexpr int64_t index = 0;
+    std::map<std::string, int64_t> zones;
+    const std::map<std::string, int64_t> expected_zones = {
+        {"a", index},
+    };
+
+    EXPECT_THAT(setZoneIndex("a", zones, index), Eq(index));
+    EXPECT_THAT(zones, ContainerEq(expected_zones));
+}
+
+TEST(SetZoneIndexTest, ZoneNotYetAssignedEmptyListNonZeroBased)
+{
+    constexpr int64_t index = 5;
+    std::map<std::string, int64_t> zones;
+    const std::map<std::string, int64_t> expected_zones = {
+        {"a", index},
+    };
+
+    EXPECT_THAT(setZoneIndex("a", zones, index), Eq(index));
+    EXPECT_THAT(zones, ContainerEq(expected_zones));
+}
+
+TEST(SetZoneIndexTest, ZoneListNotEmptyAssignsNextIndexZeroBased)
+{
+    std::map<std::string, int64_t> zones = {
+        {"a", 0},
+    };
+    const std::map<std::string, int64_t> expected_zones = {
+        {"a", 0},
+        {"b", 1},
+    };
+
+    EXPECT_THAT(setZoneIndex("b", zones, 0), Eq(1));
+    EXPECT_THAT(zones, ContainerEq(expected_zones));
+}
+
+TEST(SetZoneIndexTest, ZoneListNotEmptyAssignsNextIndexNonZeroBased)
+{
+    std::map<std::string, int64_t> zones = {
+        {"a", 0},
+    };
+    const std::map<std::string, int64_t> expected_zones = {
+        {"a", 0},
+        {"b", 5},
+    };
+
+    EXPECT_THAT(setZoneIndex("b", zones, 5), Eq(5));
+    EXPECT_THAT(zones, ContainerEq(expected_zones));
+}
+
+TEST(SetZoneIndexTest, ZoneListNotEmptyAssignsIntoGap)
+{
+    std::map<std::string, int64_t> zones = {
+        {"a", 0},
+        {"b", 5},
+    };
+    const std::map<std::string, int64_t> expected_zones = {
+        {"a", 0},
+        {"c", 1},
+        {"b", 5},
+    };
+
+    EXPECT_THAT(setZoneIndex("c", zones, 0), Eq(1));
+    EXPECT_THAT(zones, ContainerEq(expected_zones));
+}
+
 } // namespace
 } // namespace pid_control