Skip to content

Commit

Permalink
daemon: Generate transfer_id on server side
Browse files Browse the repository at this point in the history
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: #1493
  • Loading branch information
m-blaha authored and jan-kolarik committed Jun 11, 2024
1 parent 4a729aa commit 60dc52e
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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>

<!--
Expand Down
15 changes: 6 additions & 9 deletions dnf5daemon-server/services/rpm/rpm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion dnf5daemon-server/services/rpm/rpm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
13 changes: 9 additions & 4 deletions dnf5daemon-server/threads_manager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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
}
Expand Down
7 changes: 5 additions & 2 deletions doc/dnf_daemon/examples/rpm_list_fd.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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):]
Expand All @@ -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.")


Expand Down

0 comments on commit 60dc52e

Please sign in to comment.