From 81787036f2758fb23226d8b62dfd034ecea55ba0 Mon Sep 17 00:00:00 2001
From: Marek Blaha <mblaha@redhat.com>
Date: Wed, 29 May 2024 10:35:53 +0200
Subject: [PATCH] daemon: Generate transfer_id on server side

The list_fd() method of the Rpm interface uses a unique transfer_id to
identify which particular file descriptor transfer has finished.
Previously, it was the client's responsibility to pass this unique
identifier to the server in the list_fd() method call. With this patch,
the identifier is generated on the server side, simplifying the method's
usage for clients.

Resolves: https://github.com/rpm-software-management/dnf5/issues/1493
---
 .../dbus/interfaces/org.rpm.dnf.v0.rpm.Rpm.xml    |  2 +-
 dnf5daemon-server/services/rpm/rpm.cpp            | 15 ++++++---------
 dnf5daemon-server/services/rpm/rpm.hpp            |  2 +-
 dnf5daemon-server/threads_manager.hpp             | 13 +++++++++----
 doc/dnf_daemon/examples/rpm_list_fd.py            |  7 +++++--
 5 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/dnf5daemon-server/dbus/interfaces/org.rpm.dnf.v0.rpm.Rpm.xml b/dnf5daemon-server/dbus/interfaces/org.rpm.dnf.v0.rpm.Rpm.xml
index e0b2266d0..d39d9477e 100644
--- a/dnf5daemon-server/dbus/interfaces/org.rpm.dnf.v0.rpm.Rpm.xml
+++ b/dnf5daemon-server/dbus/interfaces/org.rpm.dnf.v0.rpm.Rpm.xml
@@ -107,7 +107,7 @@ along with libdnf.  If not, see <https://www.gnu.org/licenses/>.
     <method name="list_fd">
         <arg name="options" type="a{sv}" direction="in"/>
         <arg name="file_descriptor" type="h" direction="in"/>
-        <arg name="transfer_id" type="s" direction="in"/>
+        <arg name="transfer_id" type="s" direction="out"/>
     </method>
 
     <!--
diff --git a/dnf5daemon-server/services/rpm/rpm.cpp b/dnf5daemon-server/services/rpm/rpm.cpp
index d9ddebbd0..52e8e5483 100644
--- a/dnf5daemon-server/services/rpm/rpm.cpp
+++ b/dnf5daemon-server/services/rpm/rpm.cpp
@@ -66,10 +66,10 @@ void Rpm::dbus_register() {
     dbus_object->registerMethod(
         dnfdaemon::INTERFACE_RPM,
         "list_fd",
-        "a{sv}hs",
-        {"options", "file_descriptor", "transfer_id"},
-        "",
-        {},
+        "a{sv}h",
+        {"options", "file_descriptor"},
+        "s",
+        {"transfer_id"},
         [this](sdbus::MethodCall call) -> void {
             session.get_threads_manager().handle_method_fd(*this, &Rpm::list_fd, call, session.session_locale);
         });
@@ -418,16 +418,13 @@ sdbus::MethodReply Rpm::list(sdbus::MethodCall & call) {
     return reply;
 }
 
-void Rpm::list_fd(sdbus::MethodCall & call) {
+void Rpm::list_fd(sdbus::MethodCall & call, const std::string & transfer_id) {
     // read options from dbus call
     dnfdaemon::KeyValueMap options;
     call >> options;
     // get the output pipe file descriptor
     sdbus::UnixFd dbus_unix_fd;
     call >> dbus_unix_fd;
-    // id of the output filedescriptor
-    std::string out_fd_id;
-    call >> out_fd_id;
 
     int out_fd = dbus_unix_fd.get();
 
@@ -462,7 +459,7 @@ void Rpm::list_fd(sdbus::MethodCall & call) {
     auto dbus_object = get_session().get_dbus_object();
     auto signal = dbus_object->createSignal(call.getInterfaceName(), dnfdaemon::SIGNAL_WRITE_TO_FD_FINISHED);
     signal << error_msg.empty();
-    signal << out_fd_id;
+    signal << transfer_id;
     signal << error_msg;
     try {
         dbus_object->emitSignal(signal);
diff --git a/dnf5daemon-server/services/rpm/rpm.hpp b/dnf5daemon-server/services/rpm/rpm.hpp
index 615dfcb45..4809b425f 100644
--- a/dnf5daemon-server/services/rpm/rpm.hpp
+++ b/dnf5daemon-server/services/rpm/rpm.hpp
@@ -44,7 +44,7 @@ class Rpm : public IDbusSessionService {
     sdbus::MethodReply downgrade(sdbus::MethodCall & call);
     sdbus::MethodReply reinstall(sdbus::MethodCall & call);
 
-    void list_fd(sdbus::MethodCall & call);
+    void list_fd(sdbus::MethodCall & call, const std::string & transfer_id);
 };
 
 #endif
diff --git a/dnf5daemon-server/threads_manager.hpp b/dnf5daemon-server/threads_manager.hpp
index a2caca8ee..cef60f0de 100644
--- a/dnf5daemon-server/threads_manager.hpp
+++ b/dnf5daemon-server/threads_manager.hpp
@@ -108,22 +108,27 @@ class ThreadsManager {
     template <class S>
     void handle_method_fd(
         S & service,
-        void (S::*method)(sdbus::MethodCall &),
+        void (S::*method)(sdbus::MethodCall &, const std::string &),
         sdbus::MethodCall & call,
         std::optional<std::string> thread_locale = std::nullopt) {
         auto worker = std::thread(
             [this](
                 S & service,
-                void (S::*method)(sdbus::MethodCall &),
+                void (S::*method)(sdbus::MethodCall &, const std::string &),
                 sdbus::MethodCall call,
                 std::optional<std::string> thread_locale = std::nullopt) {
+                static unsigned int counter{0};
                 locale_t new_locale{nullptr};
                 locale_t orig_locale{nullptr};
                 if (thread_locale) {
                     orig_locale = set_thread_locale(thread_locale.value(), new_locale);
                 }
 
-                const sdbus::MethodReply reply = call.createReply();
+                sdbus::MethodReply reply = call.createReply();
+                // generate unique transfer id based on client bus name and counter
+                const std::string transfer_id = fmt::format("{}-{}", call.getSender(), ++counter);
+                reply << transfer_id;
+
                 std::string error_msg;
                 try {
                     reply.send();
@@ -135,7 +140,7 @@ class ThreadsManager {
 
                 if (error_msg.empty()) {
                     try {
-                        (service.*method)(call);
+                        (service.*method)(call, transfer_id);
                     } catch (...) {
                         // TODO(mblaha): log the error
                     }
diff --git a/doc/dnf_daemon/examples/rpm_list_fd.py b/doc/dnf_daemon/examples/rpm_list_fd.py
index 7caedad8a..8926f3ca2 100644
--- a/doc/dnf_daemon/examples/rpm_list_fd.py
+++ b/doc/dnf_daemon/examples/rpm_list_fd.py
@@ -40,7 +40,8 @@ def repoquery(iface_rpm, options):
     pipe_r, pipe_w = os.pipe()
     # transfer id serves as an identifier of the pipe transfer for a signal emitted
     # after server finish. This example does not use it.
-    iface_rpm.list_fd(options, pipe_w, "the_transfer_id")
+    transfer_id = iface_rpm.list_fd(options, pipe_w)
+    print("transfer_id: ", transfer_id)
     # close the write end - otherwise poll cannot detect the end of transmission
     os.close(pipe_w)
 
@@ -89,6 +90,8 @@ def repoquery(iface_rpm, options):
             try:
                 # skip all chars till begin of next JSON objects (new lines mostly)
                 json_obj_start = to_parse.find('{')
+                if json_obj_start < 0:
+                    break
                 obj, end = parser.raw_decode(to_parse[json_obj_start:])
                 yield obj
                 to_parse = to_parse[(json_obj_start+end):]
@@ -108,7 +111,7 @@ def repoquery(iface_rpm, options):
 
     # non-empty to_parse here means there are some unfinished (or generally unparsable)
     # JSON objects in the stream.
-    if to_parse:
+    if to_parse.strip():
         raise Exception("Failed to parse part of received data.")