Fixed dbus systemd interface

[Issue#] N/A
[Feature/Bug] N/A
[Problem] Services were not properly restarted. DBusAccess was waiting forever
for systemd job completion.
[Cause] Signal filter was set up too late
[Solution] Signal filter is set up before sending requests to systemd.

[Verification] Run CCMode tests.

Change-Id: I2e71a54ba44ddf0878a065e3b3bb8a7d117a6e3b

diff --git a/tests/common/dbus_access.cpp b/tests/common/dbus_access.cpp
index 0ba91f7..7251771 100644 (file)
--- a/tests/common/dbus_access.cpp
+++ b/tests/common/dbus_access.cpp
@@ -52,30 +52,30 @@ void DBusAccess::restart() {
      *  /org/freedesktop/systemd1/unit/central_2dkey_2dmanager_2eservice
      *  org.freedesktop.systemd1.Unit.Restart string:fail
      */
+    m_impl->addFilter();
     m_impl->newMethodCall(RESTART);
     m_impl->setMode(MODE_FAIL);
     m_impl->sendCommand();
-
     m_impl->waitForJobEnd();
 
     sendResetFailed();
 }
 
 void DBusAccess::start() {
+    m_impl->addFilter();
     m_impl->newMethodCall(START);
     m_impl->setMode(MODE_FAIL);
     m_impl->sendCommand();
-
     m_impl->waitForJobEnd();
 
     sendResetFailed();
 }
 
 void DBusAccess::stop() {
+    m_impl->addFilter();
     m_impl->newMethodCall(STOP);
     m_impl->setMode(MODE_FAIL);
     m_impl->sendCommand();
-
     m_impl->waitForJobEnd();
 
     sendResetFailed();

diff --git a/tests/common/dbus_access_impl.cpp b/tests/common/dbus_access_impl.cpp
index f828b30..1adaf4f 100644 (file)
--- a/tests/common/dbus_access_impl.cpp
+++ b/tests/common/dbus_access_impl.cpp
@@ -23,6 +23,18 @@
 
 #include <tests_common.h>
 
+namespace {
+void popArgument(DBusMessageIter *iter, int expected, void *value = NULL) {
+    int type = dbus_message_iter_get_arg_type(iter);
+    RUNNER_ASSERT_MSG(type == expected,
+                         "Argument type is: " << type << ". Expected: " << expected);
+
+    if (value != NULL)
+        dbus_message_iter_get_basic(iter, value);
+    dbus_message_iter_next (iter);
+}
+} // namespace anonymous
+
 DBusAccess::Impl::Impl(const char* object)
   : m_conn(NULL)
   , m_msg(NULL)
@@ -108,12 +120,8 @@ void DBusAccess::Impl::handleMsgReply() {
 
     RUNNER_ASSERT_MSG(dbus_message_iter_init(m_msg, &iter) != 0,
         "Message has no arguments");
-    if (DBUS_TYPE_OBJECT_PATH == dbus_message_iter_get_arg_type(&iter)) {
-        dbus_message_iter_get_basic(&iter, &object_path);
-        m_jobID = std::strrchr(object_path, '/') + 1;
-    } else {
-        RUNNER_ASSERT_MSG(false, "No job path in msg");
-    }
+    popArgument(&iter, DBUS_TYPE_OBJECT_PATH, &object_path);
+    m_jobID = std::strrchr(object_path, '/') + 1;
     dbus_message_unref(m_msg);
     dbus_pending_call_unref(m_pending);
     m_msg = NULL;
@@ -121,36 +129,31 @@ void DBusAccess::Impl::handleMsgReply() {
 }
 
 DBusHandlerResult DBusAccess::Impl::signalFilter(DBusConnection *, DBusMessage *message, void *This) {
-    DBusMessageIter iter;
-    dbus_uint32_t id;
     DBusAccess::Impl *a = reinterpret_cast<DBusAccess::Impl *>(This);
 
     if (dbus_message_is_signal(message, a->m_signal_interface.c_str(), a->m_signal_member.c_str()))
     {
-        RUNNER_ASSERT_MSG(dbus_message_iter_init(message, &iter) != 0,
-            "No messages in reply");
+        DBusMessageIter iter;
+        dbus_uint32_t id;
+        const char *result;
 
-        RUNNER_ASSERT_MSG(dbus_message_iter_get_arg_type(&iter) == DBUS_TYPE_UINT32,
-            "Argument is not integer");
+        dbus_message_iter_init(message, &iter);
 
-        dbus_message_iter_get_basic(&iter, &id);
+        popArgument(&iter, DBUS_TYPE_UINT32, &id);
+        popArgument(&iter, DBUS_TYPE_OBJECT_PATH);
+        popArgument(&iter, DBUS_TYPE_STRING);
+        popArgument(&iter, DBUS_TYPE_STRING, &result);
 
         if (id == (unsigned int)std::stoi(a->m_jobID)) {
-            while (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_INVALID) {
-                if (dbus_message_iter_get_arg_type(&iter) == DBUS_TYPE_STRING) {
-                    char *str = NULL;
-                    dbus_message_iter_get_basic(&iter, &str);
-                    if (!strncmp(str, "done", strlen("done")))
-                        a->m_handled = true;
-                }
-                dbus_message_iter_next(&iter);
-            }
+            RUNNER_ASSERT_MSG(strcmp(result, "done") == 0 || strcmp(result, "canceled") == 0,
+                              "Unexpected result: " << result);
+            a->m_handled = true;
         }
     }
     return (a->m_handled ? DBUS_HANDLER_RESULT_HANDLED : DBUS_HANDLER_RESULT_NOT_YET_HANDLED);
 }
 
-void DBusAccess::Impl::waitForJobEnd() {
+void DBusAccess::Impl::addFilter() {
     std::string signal_match = "type='" + m_signal_type + "',interface='" + m_signal_interface +
                                "',member='" + m_signal_member + "',path='" + m_signal_path + "'";
 
@@ -158,12 +161,10 @@ void DBusAccess::Impl::waitForJobEnd() {
     RUNNER_ASSERT_MSG(dbus_error_is_set(&m_err) != 1,
         "Error in dbus_bus_add_match: " << m_err.message);
 
-    dbus_connection_add_filter(m_conn, signalFilter, reinterpret_cast<void *>(this), NULL);
+    // TODO refactor whole class to allow multiple requests and proper filters management
+    dbus_bool_t ret = dbus_connection_add_filter(m_conn, signalFilter, reinterpret_cast<void *>(this), NULL);
+    RUNNER_ASSERT_MSG( TRUE == ret, "dbus_connection_add_filter failed");
 
-    while(dbus_connection_read_write_dispatch(m_conn, 1000)) {
-        if(m_handled)
-            break;
-    }
     m_handled = false;
 }
 
@@ -173,3 +174,11 @@ void DBusAccess::Impl::sendCommand(bool handleReply) {
     if (handleReply)
         handleMsgReply();
 }
+
+void DBusAccess::Impl::waitForJobEnd() {
+    // TODO add custom timeout
+    while(dbus_connection_read_write_dispatch(m_conn, 1000)) {
+         if(m_handled)
+             break;
+     }
+}

diff --git a/tests/common/dbus_access_impl.h b/tests/common/dbus_access_impl.h
index 8f3610a..bb4b1d9 100644 (file)
--- a/tests/common/dbus_access_impl.h
+++ b/tests/common/dbus_access_impl.h
@@ -36,9 +36,10 @@ public:
     void newMethodCall(const char* method);
     void setMode(const char* mode);
     void sendCommand(bool handleReply = true);
-    void waitForJobEnd();
+    void addFilter();
     // Systemd will not allow to many commands on single unit. This will reset command counter
     void sendResetFailed();
+    void waitForJobEnd();
 
 private:
     void connect();