Removed node name duplication in D-Bus API

Fixes: #916 The structures returned by the ListUnits and ListUnitFiles API on the Controller interface repeated the node name for each unit (file) entry. This can be reduced by using a dictionary with the node name as key and the unit (file) information in the array. Signed-off-by: Michael Engel <[email protected]>
eclipse-bluechi · Jan 3, 2025 · a99ae4e · a99ae4e
1 parent 7f73321
commit a99ae4e
Show file tree

Hide file tree

Showing 11 changed files with 217 additions and 226 deletions.
diff --git a/data/org.eclipse.bluechi.Controller.xml b/data/org.eclipse.bluechi.Controller.xml
@@ -16,8 +16,7 @@
 
     <!--
       ListUnits:
-      @units: A list of all units on each node:
-        - The node name
+      @units: A dictionary for all nodes with the respective name and a list of all units on it:
         - The primary unit name as string
         - The human readable description string
         - The load state (i.e. whether the unit file has been loaded successfully)
@@ -32,19 +31,18 @@
       List all loaded systemd units on all nodes which are online.
     -->
     <method name="ListUnits">
-      <arg name="units" type="a(sssssssouso)" direction="out" />
+      <arg name="units" type="a{sa(ssssssouso)}" direction="out" />
     </method>
 
     <!--
       ListUnitFiles:
-      @unitfiles: A list of all unit files on each node:
-        - The node name
+      @unitfiles: A dictionary for all nodes with the respective name and a list of all unit files on it:
         - The unit file path as string
         - The enabled state (i.e. whether the unit is currently enabled or not)
       List all systemd unit files.
     -->
     <method name="ListUnitFiles">
-      <arg name="unitfiles" type="a(sss)" direction="out" />
+      <arg name="unitfiles" type="a{sa(ss)}" direction="out" />
     </method>
 
     <!--

diff --git a/doc/api-examples/python/list-active-services.py b/doc/api-examples/python/list-active-services.py
@@ -13,7 +13,6 @@
 NodeUnitInfo = namedtuple(
     "NodeUnitInfo",
     [
-        "node",
         "name",
         "description",
         "load_state",
@@ -29,7 +28,8 @@
 
 controller = bus.get_proxy("org.eclipse.bluechi", "/org/eclipse/bluechi")
 units = controller.ListUnits()
-for u in units:
-    info = NodeUnitInfo(*u)
-    if info.active_state == "active" and info.name.endswith(".service"):
-        print(f"Node: {info.node}, Unit: {info.name}")
+for node, units in units.items():
+    for u in units:
+        info = NodeUnitInfo(*u)
+        if info.active_state == "active" and info.name.endswith(".service"):
+            print(f"Node: {node}, Unit: {info.name}")
diff --git a/doc/docs/api/description.md b/doc/docs/api/description.md
@@ -16,17 +16,17 @@ Note that some properties also come with change events, so you can easily track
 
 #### Methods
 
-  * `ListUnits(out a(sssssssouso) units)`
+  * `ListUnits(out a{sa(ssssssouso)} units)`
 
-    Returns an array with all currently loaded systemd units on all online nodes. This is equivalent to calling
-    `Node.ListUnits()` on all the online nodes and adding the name of the node as the first element of each returned
-    element struct.
+    Returns a dictionary with all online nodes and all currently loaded systemd units on them. This is equivalent to calling
+    `Node.ListUnits()` on all the online nodes and adding the name of the node as the key element of the returned
+    dictionary.
 
-  * `ListUnitFiles(out a(sss) unit_files)`
+  * `ListUnitFiles(out a{sa(ss)} unit_files)`
 
-    Returns an array with all systemd unit files on all online nodes. This is equivalent to calling
-    `Node.ListUnitFiles()` on all the online nodes and adding the name of the node as the first element of each returned
-    element struct.
+    Returns a dictionary with all online nodes and all systemd unit files on them. This is equivalent to calling
+    `Node.ListUnitFiles()` on all the online nodes and adding the name of the node as the key element of the returned
+    dictionary.
 
   * `CreateMonitor(out o monitor)`
 

diff --git a/src/bindings/python/bluechi/api.py b/src/bindings/python/bluechi/api.py
@@ -325,11 +325,10 @@ def list_nodes(self) -> List[Tuple[str, ObjPath, str, str]]:
         """
         return self.get_proxy().ListNodes()
 
-    def list_unit_files(self) -> List[Tuple[str, str, str]]:
+    def list_unit_files(self) -> Dict[str, List[Tuple[str, str]]]:
         """
           ListUnitFiles:
-        @unitfiles: A list of all unit files on each node:
-          - The node name
+        @unitfiles: A dictionary for all nodes with the respective name and a list of all unit files on it:
           - The unit file path as string
           - The enabled state (i.e. whether the unit is currently enabled or not)
         List all systemd unit files.
@@ -338,11 +337,12 @@ def list_unit_files(self) -> List[Tuple[str, str, str]]:
 
     def list_units(
         self,
-    ) -> List[Tuple[str, str, str, str, str, str, str, ObjPath, UInt32, str, ObjPath]]:
+    ) -> Dict[
+        str, List[Tuple[str, str, str, str, str, str, ObjPath, UInt32, str, ObjPath]]
+    ]:
         """
           ListUnits:
-        @units: A list of all units on each node:
-          - The node name
+        @units: A dictionary for all nodes with the respective name and a list of all units on it:
           - The primary unit name as string
           - The human readable description string
           - The load state (i.e. whether the unit file has been loaded successfully)

diff --git a/src/bindings/python/setup.py b/src/bindings/python/setup.py
@@ -12,7 +12,7 @@ def readme():
 
 setup(
     name="bluechi",
-    version="0.9.0",
+    version="0.10.0",
     description="Python bindings for BlueChi's D-Bus API",
     long_description=readme(),
     long_description_content_type="text/markdown",

diff --git a/src/client/method-list-unit-files.c b/src/client/method-list-unit-files.c
@@ -31,74 +31,93 @@ void unit_file_list_unref(UnitFileList *unit_file_list);
 DEFINE_CLEANUP_FUNC(UnitFileList, unit_file_list_unref)
 #define _cleanup_unit_file_list_ _cleanup_(unit_file_list_unrefp)
 
-static int fetch_unit_file_list(
-                sd_bus *api_bus,
-                const char *node_name,
-                const char *object_path,
-                const char *interface,
-                const char *typestring,
-                int (*parse_unit_file_info)(sd_bus_message *, UnitFileInfo *),
-                UnitFileList *unit_file_list) {
-        int r = 0;
-        _cleanup_sd_bus_error_ sd_bus_error error = SD_BUS_ERROR_NULL;
-        _cleanup_sd_bus_message_ sd_bus_message *message = NULL;
 
-        r = sd_bus_call_method(
-                        api_bus,
-                        BC_INTERFACE_BASE_NAME,
-                        object_path,
-                        interface,
-                        "ListUnitFiles",
-                        &error,
-                        &message,
-                        "");
-        if (r < 0) {
-                fprintf(stderr, "Failed to issue method call: %s\n", error.message);
-                return r;
-        }
+static int parse_unit_file_list(sd_bus_message *message, const char *node_name, UnitFileList *unit_file_list) {
 
-        r = sd_bus_message_enter_container(message, SD_BUS_TYPE_ARRAY, typestring);
+        int r = 0;
+        r = sd_bus_message_enter_container(message, SD_BUS_TYPE_ARRAY, UNIT_FILE_INFO_STRUCT_TYPESTRING);
         if (r < 0) {
-                fprintf(stderr, "Failed to read sd-bus message: %s\n", strerror(-r));
+                fprintf(stderr, "Failed to enter sd-bus message container: %s\n", strerror(-r));
                 return r;
         }
 
         for (;;) {
                 _cleanup_unit_file_ UnitFileInfo *info = new_unit_file();
 
-                r = (*parse_unit_file_info)(message, info);
+                r = bus_parse_unit_file_info(message, info);
                 if (r < 0) {
-                        fprintf(stderr, "Failed to parse unit file info: %s\n", strerror(-r));
+                        fprintf(stderr, "Failed to parse unit info: %s\n", strerror(-r));
                         return r;
                 }
                 if (r == 0) {
                         break;
                 }
-                if (node_name != NULL) {
-                        info->node = strdup(node_name);
-                }
+                info->node = strdup(node_name);
 
                 LIST_APPEND(unit_files, unit_file_list->unit_files, unit_file_ref(info));
         }
 
+        r = sd_bus_message_exit_container(message);
+        if (r < 0) {
+                fprintf(stderr, "Failed to exit sd-bus message container: %s\n", strerror(-r));
+                return r;
+        }
+
         return r;
 }
 
 static int method_list_unit_files_on_all(sd_bus *api_bus, print_unit_file_list_fn print, const char *glob_filter) {
+        int r = 0;
         _cleanup_unit_file_list_ UnitFileList *unit_file_list = new_unit_file_list();
 
-        int r = fetch_unit_file_list(
+        _cleanup_sd_bus_error_ sd_bus_error error = SD_BUS_ERROR_NULL;
+        _cleanup_sd_bus_message_ sd_bus_message *message = NULL;
+        r = sd_bus_call_method(
                         api_bus,
-                        NULL,
+                        BC_INTERFACE_BASE_NAME,
                         BC_OBJECT_PATH,
                         CONTROLLER_INTERFACE,
-                        NODE_AND_UNIT_FILE_INFO_STRUCT_TYPESTRING,
-                        &bus_parse_unit_file_on_node_info,
-                        unit_file_list);
+                        "ListUnitFiles",
+                        &error,
+                        &message,
+                        "");
         if (r < 0) {
+                fprintf(stderr, "Failed to issue method call: %s\n", error.message);
                 return r;
         }
 
+        r = sd_bus_message_enter_container(message, SD_BUS_TYPE_ARRAY, NODE_AND_UNIT_FILE_INFO_DICT_TYPESTRING);
+        if (r < 0) {
+                fprintf(stderr, "Failed to read sd-bus message: %s\n", strerror(-r));
+                return r;
+        }
+
+        for (;;) {
+                r = sd_bus_message_enter_container(
+                                message, SD_BUS_TYPE_DICT_ENTRY, NODE_AND_UNIT_FILE_INFO_TYPESTRING);
+                if (r < 0) {
+                        fprintf(stderr, "Failed to enter sd-bus message dictionary: %s\n", strerror(-r));
+                        return r;
+                }
+                if (r == 0) {
+                        break;
+                }
+
+                char *node_name = NULL;
+                r = sd_bus_message_read(message, "s", &node_name);
+                if (r < 0) {
+                        fprintf(stderr, "Failed to read node name: %s\n", strerror(-r));
+                        return r;
+                }
+                parse_unit_file_list(message, node_name, unit_file_list);
+
+                r = sd_bus_message_exit_container(message);
+                if (r < 0) {
+                        fprintf(stderr, "Failed to exit sd-bus message dictionary: %s\n", strerror(-r));
+                        return r;
+                }
+        }
+
         print(unit_file_list, glob_filter);
 
         return 0;
@@ -115,14 +134,23 @@ static int method_list_unit_files_on(
                 return r;
         }
 
-        r = fetch_unit_file_list(
+        _cleanup_sd_bus_error_ sd_bus_error error = SD_BUS_ERROR_NULL;
+        _cleanup_sd_bus_message_ sd_bus_message *message = NULL;
+        r = sd_bus_call_method(
                         api_bus,
-                        node_name,
+                        BC_INTERFACE_BASE_NAME,
                         object_path,
                         NODE_INTERFACE,
-                        UNIT_FILE_INFO_STRUCT_TYPESTRING,
-                        &bus_parse_unit_file_info,
-                        unit_file_list);
+                        "ListUnitFiles",
+                        &error,
+                        &message,
+                        "");
+        if (r < 0) {
+                fprintf(stderr, "Failed to issue method call: %s\n", error.message);
+                return r;
+        }
+
+        r = parse_unit_file_list(message, node_name, unit_file_list);
         if (r < 0) {
                 return r;
         }