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>
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 @@
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 @@
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 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 @@
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;
+}