unit-test: Test deleting entry on name change

Breaking off into a separate function enables easier unit testing of the
specific function

Testing: 97% coverage of processing.cpp

Change-Id: I08f229657a8f44230b711fabbae20fb403792637
Signed-off-by: Andrew Geissler <geissonator@yahoo.com>

6 files changed, 168 insertions, 56 deletions

diff --git a/src/main.cpp b/src/main.cpp
index e96ffe3..e5d566e 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -15,19 +15,9 @@
 
 constexpr const char* OBJECT_MAPPER_DBUS_NAME =
     "xyz.openbmc_project.ObjectMapper";
-constexpr const char* ASSOCIATIONS_INTERFACE = "org.openbmc.Associations";
 constexpr const char* XYZ_ASSOCIATION_INTERFACE =
     "xyz.openbmc_project.Association";
 
-// interface_map_type is the underlying datastructure the mapper uses.
-// The 3 levels of map are
-// object paths
-//   connection names
-//      interface names
-using interface_map_type = boost::container::flat_map<
-    std::string, boost::container::flat_map<
-                     std::string, boost::container::flat_set<std::string>>>;
-
 using Association = std::tuple<std::string, std::string, std::string>;
 
 AssociationInterfaces associationInterfaces;
@@ -660,54 +650,17 @@ int main(int argc, char** argv)
     std::function<void(sdbusplus::message::message & message)>
         nameChangeHandler = [&interface_map, &io, &name_owners, &server,
                              system_bus](sdbusplus::message::message& message) {
-            std::string name;
-            std::string old_owner;
-            std::string new_owner;
+            std::string name;      // well-known
+            std::string old_owner; // unique-name
+            std::string new_owner; // unique-name
 
             message.read(name, old_owner, new_owner);
 
             if (!old_owner.empty())
             {
-                if (boost::starts_with(old_owner, ":"))
-                {
-                    auto it = name_owners.find(old_owner);
-                    if (it != name_owners.end())
-                    {
-                        name_owners.erase(it);
-                    }
-                }
-                // Connection removed
-                interface_map_type::iterator path_it = interface_map.begin();
-                while (path_it != interface_map.end())
-                {
-                    // If an associations interface is being removed,
-                    // also need to remove the corresponding associations
-                    // objects and properties.
-                    auto ifaces = path_it->second.find(name);
-                    if (ifaces != path_it->second.end())
-                    {
-                        auto assoc = std::find(ifaces->second.begin(),
-                                               ifaces->second.end(),
-                                               ASSOCIATIONS_INTERFACE);
-
-                        if (assoc != ifaces->second.end())
-                        {
-                            removeAssociation(path_it->first, name, server,
-                                              associationOwners,
-                                              associationInterfaces);
-                        }
-                    }
-
-                    path_it->second.erase(name);
-                    if (path_it->second.empty())
-                    {
-                        // If the last connection to the object is gone,
-                        // delete the top level object
-                        path_it = interface_map.erase(path_it);
-                        continue;
-                    }
-                    path_it++;
-                }
+                processNameChangeDelete(name_owners, name, old_owner,
+                                        interface_map, associationOwners,
+                                        associationInterfaces, server);
             }
 
             if (!new_owner.empty())
diff --git a/src/processing.cpp b/src/processing.cpp
index be7bf12..3cd348b 100644
--- a/src/processing.cpp
+++ b/src/processing.cpp
@@ -37,3 +37,48 @@ bool needToIntrospect(const std::string& processName,
 
     return inWhitelist && !inBlacklist;
 }
+
+void processNameChangeDelete(
+    boost::container::flat_map<std::string, std::string>& nameOwners,
+    const std::string& wellKnown, const std::string& oldOwner,
+    interface_map_type& interfaceMap, AssociationOwnersType& assocOwners,
+    AssociationInterfaces& assocInterfaces,
+    sdbusplus::asio::object_server& server)
+{
+    if (boost::starts_with(oldOwner, ":"))
+    {
+        auto it = nameOwners.find(oldOwner);
+        if (it != nameOwners.end())
+        {
+            nameOwners.erase(it);
+        }
+    }
+    // Connection removed
+    interface_map_type::iterator pathIt = interfaceMap.begin();
+    while (pathIt != interfaceMap.end())
+    {
+        // If an associations interface is being removed,
+        // also need to remove the corresponding associations
+        // objects and properties.
+        auto ifaces = pathIt->second.find(wellKnown);
+        if (ifaces != pathIt->second.end())
+        {
+            auto assoc = std::find(ifaces->second.begin(), ifaces->second.end(),
+                                   ASSOCIATIONS_INTERFACE);
+            if (assoc != ifaces->second.end())
+            {
+                removeAssociation(pathIt->first, wellKnown, server, assocOwners,
+                                  assocInterfaces);
+            }
+        }
+        pathIt->second.erase(wellKnown);
+        if (pathIt->second.empty())
+        {
+            // If the last connection to the object is gone,
+            // delete the top level object
+            pathIt = interfaceMap.erase(pathIt);
+            continue;
+        }
+        pathIt++;
+    }
+}
diff --git a/src/processing.hpp b/src/processing.hpp
index 00f8a6d..8ef6e48 100644
--- a/src/processing.hpp
+++ b/src/processing.hpp
@@ -1,5 +1,7 @@
 #pragma once
 
+#include "associations.hpp"
+
 #include <boost/container/flat_map.hpp>
 #include <boost/container/flat_set.hpp>
 #include <string>
@@ -7,6 +9,20 @@
 /** @brief Define white list and black list data structure */
 using WhiteBlackList = boost::container::flat_set<std::string>;
 
+/** @brief Dbus interface which contains org.openbmc Associations */
+constexpr const char* ASSOCIATIONS_INTERFACE = "org.openbmc.Associations";
+
+/** @brief interface_map_type is the underlying datastructure the mapper uses.
+ *
+ * The 3 levels of map are
+ * object paths
+ *   connection names
+ *      interface names
+ */
+using interface_map_type = boost::container::flat_map<
+    std::string, boost::container::flat_map<
+                     std::string, boost::container::flat_set<std::string>>>;
+
 /** @brief Get well known name of input unique name
  *
  * If user passes in well known name then that will be returned.
@@ -36,3 +52,21 @@ bool getWellKnown(
 bool needToIntrospect(const std::string& processName,
                       const WhiteBlackList& whiteList,
                       const WhiteBlackList& blackList);
+
+/** @brief Handle the removal of an existing name in objmgr data structures
+ *
+ * @param[in,out] nameOwners      - Map of unique name to well known name
+ * @param[in]     wellKnown       - Well known name that has new owner
+ * @param[in]     oldOwner        - Old unique name
+ * @param[in,out] interfaceMap    - Map of interfaces
+ * @param[in,out] assocOwners     - Owners of associations
+ * @param[in,out] assocInterfaces - Associations endpoints
+ * @param[in,out] server          - sdbus system object
+ *
+ */
+void processNameChangeDelete(
+    boost::container::flat_map<std::string, std::string>& nameOwners,
+    const std::string& wellKnown, const std::string& oldOwner,
+    interface_map_type& interfaceMap, AssociationOwnersType& assocOwners,
+    AssociationInterfaces& assocInterfaces,
+    sdbusplus::asio::object_server& server);
diff --git a/src/test/Makefile.am.include b/src/test/Makefile.am.include
index b54ddbf..394c610 100644
--- a/src/test/Makefile.am.include
+++ b/src/test/Makefile.am.include
@@ -1,12 +1,17 @@
-src_test_well_known_SOURCES = %reldir%/well_known.cpp src/processing.cpp
+src_test_well_known_SOURCES = %reldir%/well_known.cpp src/processing.cpp \
+	src/associations.cpp
 
 src_test_need_to_introspect_SOURCES = %reldir%/need_to_introspect.cpp \
-	src/processing.cpp
+	src/processing.cpp src/associations.cpp
 
 src_test_associations_SOURCES = %reldir%/associations.cpp \
     src/associations.cpp
 
+src_test_name_change_SOURCES = %reldir%/name_change.cpp \
+		src/associations.cpp src/processing.cpp
+
 check_PROGRAMS += \
 	%reldir%/well_known \
 	%reldir%/need_to_introspect \
-	%reldir%/associations
+	%reldir%/associations \
+	%reldir%/name_change
diff --git a/src/test/name_change.cpp b/src/test/name_change.cpp
new file mode 100644
index 0000000..f4a9662
--- /dev/null
+++ b/src/test/name_change.cpp
@@ -0,0 +1,62 @@
+#include "src/processing.hpp"
+#include "src/test/util/asio_server_class.hpp"
+#include "src/test/util/association_objects.hpp"
+
+#include <gtest/gtest.h>
+
+class TestNameChange : public AsioServerClassTest
+{
+};
+sdbusplus::asio::object_server* TestNameChange::AsioServerClassTest::server =
+    nullptr;
+
+// Verify unique name is removed from nameOwners
+TEST_F(TestNameChange, UniqueNameNoInterfaces)
+{
+    boost::container::flat_map<std::string, std::string> nameOwners = {
+        {":1.99", "test-name"}};
+    std::string wellKnown = {"test-name"};
+    std::string oldOwner = {":1.99"};
+    interface_map_type interfaceMap;
+    AssociationOwnersType assocOwners;
+    AssociationInterfaces assocInterfaces;
+
+    processNameChangeDelete(nameOwners, wellKnown, oldOwner, interfaceMap,
+                            assocOwners, assocInterfaces, *server);
+    EXPECT_EQ(nameOwners.size(), 0);
+}
+
+// Verify path removed from interface map and association objects
+TEST_F(TestNameChange, UniqueNameAssociationsAndInterface)
+{
+    boost::container::flat_map<std::string, std::string> nameOwners = {
+        {":1.99", DEFAULT_DBUS_SVC}};
+    std::string oldOwner = {":1.99"};
+    boost::container::flat_set<std::string> assocInterfacesSet = {
+        ASSOCIATIONS_INTERFACE};
+
+    // Build up these objects so that an associated interface will match
+    // with the associated owner being removed
+    auto assocOwners = createDefaultOwnerAssociation();
+    auto assocInterfaces = createDefaultInterfaceAssociation(server);
+    auto interfaceMap = createInterfaceMap(
+        DEFAULT_SOURCE_PATH, DEFAULT_DBUS_SVC, assocInterfacesSet);
+
+    processNameChangeDelete(nameOwners, DEFAULT_DBUS_SVC, oldOwner,
+                            interfaceMap, assocOwners, assocInterfaces,
+                            *server);
+    EXPECT_EQ(nameOwners.size(), 0);
+
+    // Verify owner association was deleted
+    EXPECT_TRUE(assocOwners.empty());
+
+    // Verify endpoint was deleted from interface association
+    auto intfEndpoints =
+        std::get<endpointsPos>(assocInterfaces[DEFAULT_FWD_PATH]);
+    EXPECT_EQ(intfEndpoints.size(), 0);
+    intfEndpoints = std::get<endpointsPos>(assocInterfaces[DEFAULT_REV_PATH]);
+    EXPECT_EQ(intfEndpoints.size(), 0);
+
+    // Verify interface map was deleted
+    EXPECT_TRUE(interfaceMap.empty());
+}
diff --git a/src/test/util/association_objects.hpp b/src/test/util/association_objects.hpp
index 081f891..45b89d6 100644
--- a/src/test/util/association_objects.hpp
+++ b/src/test/util/association_objects.hpp
@@ -1,4 +1,5 @@
 #include "src/associations.hpp"
+#include "src/processing.hpp"
 
 const std::string DEFAULT_SOURCE_PATH = "/logging/entry/1";
 const std::string DEFAULT_DBUS_SVC = "xyz.openbmc_project.New.Interface";
@@ -45,3 +46,15 @@ void addEndpointToInterfaceAssociation(AssociationInterfaces& interfaceAssoc)
     auto endpoints = std::get<endpointsPos>(iface);
     endpoints.push_back(EXTRA_ENDPOINT);
 }
+
+// Create a default interface_map_type with input values
+interface_map_type createInterfaceMap(
+    const std::string& path, const std::string& connection_name,
+    const boost::container::flat_set<std::string>& interface_names)
+{
+    boost::container::flat_map<std::string,
+                               boost::container::flat_set<std::string>>
+        connectionMap = {{connection_name, interface_names}};
+    interface_map_type interfaceMap = {{path, connectionMap}};
+    return interfaceMap;
+}