Dbus async method call handler

[Bug/Feature]   Enable deferred set of result in dbus method handler.
[Cause]         N/A
[Solution]      N/A
[Verification]  Build, install, run tests

Change-Id: I11b3abe0886bc560f8b63f6206c64695f2f7eb1a

diff --git a/common/dbus/connection.cpp b/common/dbus/connection.cpp
index 4a4a2cd..ef6fc46 100644 (file)
--- a/common/dbus/connection.cpp
+++ b/common/dbus/connection.cpp
@@ -47,6 +47,12 @@ class MethodResultBuilderImpl : public MethodResultBuilder {
 public:
     MethodResultBuilderImpl(GDBusMethodInvocation* invocation)
         : mInvocation(invocation), mResultSet(false) {}
+    ~MethodResultBuilderImpl()
+    {
+        if (!mResultSet) {
+            setError("org.freedesktop.DBus.Error.UnknownMethod", "Not implemented");
+        }
+    }
     void set(GVariant* parameters)
     {
         g_dbus_method_invocation_return_value(mInvocation, parameters);
@@ -61,10 +67,6 @@ public:
         g_dbus_method_invocation_return_dbus_error(mInvocation, name.c_str(), message.c_str());
         mResultSet = true;
     }
-    bool isUndefined() const
-    {
-        return !mResultSet;
-    }
 private:
     GDBusMethodInvocation* mInvocation;
     bool mResultSet;
@@ -305,15 +307,10 @@ void DbusConnection::onMethodCall(GDBusConnection*,
 
     LOGD("MethodCall: " << objectPath << "; " << interface << "; " << method);
 
-    MethodResultBuilderImpl resultBuilder(invocation);
+    MethodResultBuilder::Pointer resultBuilder(new MethodResultBuilderImpl(invocation));
     if (callback) {
         callback(objectPath, interface, method, parameters, resultBuilder);
     }
-
-    if (resultBuilder.isUndefined()) {
-        LOGW("Unimplemented method: " << objectPath << "; " << interface << "; " << method);
-        resultBuilder.setError("org.freedesktop.DBus.Error.UnknownMethod", "Not implemented");
-    }
 }
 
 GVariantPtr DbusConnection::callMethod(const std::string& busName,

diff --git a/common/dbus/connection.hpp b/common/dbus/connection.hpp
index 28932af..9ac87de 100644 (file)
--- a/common/dbus/connection.hpp
+++ b/common/dbus/connection.hpp
@@ -44,6 +44,8 @@ typedef std::unique_ptr<GVariant, void(*)(GVariant*)> GVariantPtr;
  */
 class MethodResultBuilder {
 public:
+    typedef std::shared_ptr<MethodResultBuilder> Pointer;
+
     virtual ~MethodResultBuilder() {}
     virtual void set(GVariant* parameters) = 0;
     virtual void setVoid() = 0;
@@ -76,7 +78,7 @@ public:
                                const std::string& interface,
                                const std::string& methodName,
                                GVariant* parameters,
-                               MethodResultBuilder& result
+                               MethodResultBuilder::Pointer result
                               )> MethodCallCallback;
 
     typedef std::function<void(const std::string& senderBusName,

diff --git a/container-daemon/daemon-connection.cpp b/container-daemon/daemon-connection.cpp
index 4f66e37..0eb6636 100644 (file)
--- a/container-daemon/daemon-connection.cpp
+++ b/container-daemon/daemon-connection.cpp
@@ -119,7 +119,7 @@ void DaemonConnection::onMessageCall(const std::string& objectPath,
                                      const std::string& interface,
                                      const std::string& methodName,
                                      GVariant* /*parameters*/,
-                                     dbus::MethodResultBuilder& result)
+                                     dbus::MethodResultBuilder::Pointer result)
 {
     if (objectPath != api::OBJECT_PATH || interface != api::INTERFACE) {
         return;
@@ -128,12 +128,12 @@ void DaemonConnection::onMessageCall(const std::string& objectPath,
     if (methodName == api::METHOD_GAIN_FOCUS) {
         if (mGainFocusCallback) {
             mGainFocusCallback();
-            result.setVoid();
+            result->setVoid();
         }
     } else if (methodName == api::METHOD_LOSE_FOCUS) {
         if (mLoseFocusCallback) {
             mLoseFocusCallback();
-            result.setVoid();
+            result->setVoid();
         }
     }
 }

diff --git a/container-daemon/daemon-connection.hpp b/container-daemon/daemon-connection.hpp
index e2e5154..34129df 100644 (file)
--- a/container-daemon/daemon-connection.hpp
+++ b/container-daemon/daemon-connection.hpp
@@ -68,7 +68,7 @@ private:
                        const std::string& interface,
                        const std::string& methodName,
                        GVariant* parameters,
-                       dbus::MethodResultBuilder& result);
+                       dbus::MethodResultBuilder::Pointer result);
 };
 
 

diff --git a/server/container-connection.cpp b/server/container-connection.cpp
index 3ef2dd7..adabace 100644 (file)
--- a/server/container-connection.cpp
+++ b/server/container-connection.cpp
@@ -150,7 +150,7 @@ void ContainerConnection::onMessageCall(const std::string& objectPath,
                                         const std::string& interface,
                                         const std::string& methodName,
                                         GVariant* parameters,
-                                        dbus::MethodResultBuilder& result)
+                                        dbus::MethodResultBuilder::Pointer result)
 {
     if (objectPath != api::OBJECT_PATH || interface != api::INTERFACE) {
         return;
@@ -162,7 +162,7 @@ void ContainerConnection::onMessageCall(const std::string& objectPath,
         g_variant_get(parameters, "(&s&s)", &application, &message);
         if (mNotifyActiveContainerCallback) {
             mNotifyActiveContainerCallback(application, message);
-            result.setVoid();
+            result->setVoid();
         }
     }
 

diff --git a/server/container-connection.hpp b/server/container-connection.hpp
index 2f429ce..7759a09 100644 (file)
--- a/server/container-connection.hpp
+++ b/server/container-connection.hpp
@@ -52,7 +52,7 @@ public:
 
     typedef std::function<void(const std::string& destination,
                                const std::string& path,
-                               dbus::MethodResultBuilder& result
+                               dbus::MethodResultBuilder::Pointer result
                               )> FileMoveRequestCallback;
 
     /**
@@ -96,7 +96,7 @@ private:
                        const std::string& interface,
                        const std::string& methodName,
                        GVariant* parameters,
-                       dbus::MethodResultBuilder& result);
+                       dbus::MethodResultBuilder::Pointer result);
     void onSignalReceived(const std::string& senderBusName,
                           const std::string& objectPath,
                           const std::string& interface,

diff --git a/server/containers-manager.cpp b/server/containers-manager.cpp
index 657c7bf..0150e4a 100644 (file)
--- a/server/containers-manager.cpp
+++ b/server/containers-manager.cpp
@@ -238,7 +238,7 @@ void ContainersManager::displayOffHandler(const std::string& /*caller*/)
 void ContainersManager::handleContainerMoveFileRequest(const std::string& srcContainerId,
                                                        const std::string& dstContainerId,
                                                        const std::string& path,
-                                                       dbus::MethodResultBuilder& result)
+                                                       dbus::MethodResultBuilder::Pointer result)
 {
     // TODO: this implementation is only a placeholder.
     // There are too many unanswered questions and security concerns:
@@ -274,26 +274,26 @@ void ContainersManager::handleContainerMoveFileRequest(const std::string& srcCon
     ContainerMap::const_iterator dstIter = mContainers.find(dstContainerId);
     if (dstIter == mContainers.end()) {
         LOGE("Destination container '" << dstContainerId << "' not found");
-        result.set(g_variant_new("(s)", api::FILE_MOVE_DESTINATION_NOT_FOUND.c_str()));
+        result->set(g_variant_new("(s)", api::FILE_MOVE_DESTINATION_NOT_FOUND.c_str()));
         return;
     }
     Container& dstContanier = *dstIter->second;
 
     if (srcContainerId == dstContainerId) {
         LOGE("Cannot send a file to yourself");
-        result.set(g_variant_new("(s)", api::FILE_MOVE_WRONG_DESTINATION.c_str()));
+        result->set(g_variant_new("(s)", api::FILE_MOVE_WRONG_DESTINATION.c_str()));
         return;
     }
 
     if (!regexMatchVector(path, srcContainer.getPermittedToSend())) {
         LOGE("Source container has no permissions to send the file: " << path);
-        result.set(g_variant_new("(s)", api::FILE_MOVE_NO_PERMISSIONS_SEND.c_str()));
+        result->set(g_variant_new("(s)", api::FILE_MOVE_NO_PERMISSIONS_SEND.c_str()));
         return;
     }
 
     if (!regexMatchVector(path, dstContanier.getPermittedToRecv())) {
         LOGE("Destination container has no permissions to receive the file: " << path);
-        result.set(g_variant_new("(s)", api::FILE_MOVE_NO_PERMISSIONS_RECEIVE.c_str()));
+        result->set(g_variant_new("(s)", api::FILE_MOVE_NO_PERMISSIONS_RECEIVE.c_str()));
         return;
     }
 
@@ -303,9 +303,9 @@ void ContainersManager::handleContainerMoveFileRequest(const std::string& srcCon
 
     if (!utils::moveFile(srcPath, dstPath)) {
         LOGE("Failed to move the file: " << path);
-        result.set(g_variant_new("(s)", api::FILE_MOVE_FAILED.c_str()));
+        result->set(g_variant_new("(s)", api::FILE_MOVE_FAILED.c_str()));
     } else {
-        result.set(g_variant_new("(s)", api::FILE_MOVE_SUCCEEDED.c_str()));
+        result->set(g_variant_new("(s)", api::FILE_MOVE_SUCCEEDED.c_str()));
         try {
             dstContanier.sendNotification(srcContainerId, path, api::FILE_MOVE_SUCCEEDED);
         } catch (ServerException&) {

diff --git a/server/containers-manager.hpp b/server/containers-manager.hpp
index 32ca341..1e29ed7 100644 (file)
--- a/server/containers-manager.hpp
+++ b/server/containers-manager.hpp
@@ -91,7 +91,7 @@ private:
     void handleContainerMoveFileRequest(const std::string& srcContainerId,
                                         const std::string& dstContainerId,
                                         const std::string& path,
-                                        dbus::MethodResultBuilder& result);
+                                        dbus::MethodResultBuilder::Pointer result);
 };
 
 

diff --git a/server/host-connection.cpp b/server/host-connection.cpp
index 4b3c59d..4f9a2f2 100644 (file)
--- a/server/host-connection.cpp
+++ b/server/host-connection.cpp
@@ -119,7 +119,7 @@ void HostConnection::onMessageCall(const std::string& objectPath,
                                         const std::string& interface,
                                         const std::string& methodName,
                                         GVariant* /*parameters*/,
-                                        dbus::MethodResultBuilder& result)
+                                        dbus::MethodResultBuilder::Pointer result)
 {
     if (objectPath != hostapi::OBJECT_PATH || interface != hostapi::INTERFACE) {
         return;
@@ -128,7 +128,7 @@ void HostConnection::onMessageCall(const std::string& objectPath,
     if (methodName == hostapi::METHOD_TEST) {
         if (mTestCallback) {
             mTestCallback();
-            result.setVoid();
+            result->setVoid();
         }
     }
 }

diff --git a/server/host-connection.hpp b/server/host-connection.hpp
index 4d7eccd..d78949f 100644 (file)
--- a/server/host-connection.hpp
+++ b/server/host-connection.hpp
@@ -66,7 +66,7 @@ private:
                        const std::string& interface,
                        const std::string& methodName,
                        GVariant* parameters,
-                       dbus::MethodResultBuilder& result);
+                       dbus::MethodResultBuilder::Pointer result);
 };
 
 

diff --git a/tests/unit_tests/dbus/test-server.cpp b/tests/unit_tests/dbus/test-server.cpp
index 711f16f..3a77390 100644 (file)
--- a/tests/unit_tests/dbus/test-server.cpp
+++ b/tests/unit_tests/dbus/test-server.cpp
@@ -117,7 +117,7 @@ void DbusTestServer::onMessageCall(const std::string& objectPath,
                                    const std::string& interface,
                                    const std::string& methodName,
                                    GVariant* parameters,
-                                   dbus::MethodResultBuilder& result)
+                                   dbus::MethodResultBuilder::Pointer result)
 {
     try {
         if (objectPath != TESTAPI_OBJECT_PATH || interface != TESTAPI_INTERFACE) {
@@ -126,23 +126,23 @@ void DbusTestServer::onMessageCall(const std::string& objectPath,
 
         if (methodName == TESTAPI_METHOD_NOOP) {
             noop();
-            result.setVoid();
+            result->setVoid();
         } else if (methodName == TESTAPI_METHOD_PROCESS) {
             const gchar* arg;
             g_variant_get(parameters, "(&s)", &arg);
             std::string ret = process(arg);
             GVariant* variant = g_variant_new("(s)", ret.c_str());
-            result.set(variant);
+            result->set(variant);
         } else if (methodName == TESTAPI_METHOD_THROW) {
             int arg;
             g_variant_get(parameters, "(i)", &arg);
             throwException(arg);
-            result.setVoid();
+            result->setVoid();
         } else {
             LOGE("unknown method; should never happen");
         }
     } catch (const std::exception& e) {
-        result.setError("org.tizen.containers.Error.Test", e.what());
+        result->setError("org.tizen.containers.Error.Test", e.what());
     }
 }
 

diff --git a/tests/unit_tests/dbus/test-server.hpp b/tests/unit_tests/dbus/test-server.hpp
index b52ebb3..8e2aa7f 100644 (file)
--- a/tests/unit_tests/dbus/test-server.hpp
+++ b/tests/unit_tests/dbus/test-server.hpp
@@ -73,7 +73,7 @@ private:
                        const std::string& interface,
                        const std::string& methodName,
                        GVariant* parameters,
-                       dbus::MethodResultBuilder& result);
+                       dbus::MethodResultBuilder::Pointer result);
 };
 
 

diff --git a/tests/unit_tests/dbus/ut-connection.cpp b/tests/unit_tests/dbus/ut-connection.cpp
index 131e359..21be1e8 100644 (file)
--- a/tests/unit_tests/dbus/ut-connection.cpp
+++ b/tests/unit_tests/dbus/ut-connection.cpp
@@ -324,21 +324,21 @@ BOOST_AUTO_TEST_CASE(MethodCallTest)
                       const std::string& interface,
                       const std::string& methodName,
                       GVariant* parameters,
-                      MethodResultBuilder& result) {
+                      MethodResultBuilder::Pointer result) {
         if (objectPath != TESTAPI_OBJECT_PATH || interface != TESTAPI_INTERFACE) {
             return;
         }
         if (methodName == TESTAPI_METHOD_NOOP) {
-            result.setVoid();
+            result->setVoid();
         } else if (methodName == TESTAPI_METHOD_PROCESS) {
             const gchar* arg = NULL;
             g_variant_get(parameters, "(&s)", &arg);
             std::string str = std::string("resp: ") + arg;
-            result.set(g_variant_new("(s)", str.c_str()));
+            result->set(g_variant_new("(s)", str.c_str()));
         } else if (methodName == TESTAPI_METHOD_THROW) {
             int arg = 0;
             g_variant_get(parameters, "(i)", &arg);
-            result.setError("org.tizen.containers.Error.Test", "msg: " + std::to_string(arg));
+            result->setError("org.tizen.containers.Error.Test", "msg: " + std::to_string(arg));
         }
     };
     conn1->registerObject(TESTAPI_OBJECT_PATH, TESTAPI_DEFINITION, handler);
@@ -387,21 +387,21 @@ BOOST_AUTO_TEST_CASE(MethodAsyncCallTest)
                       const std::string& interface,
                       const std::string& methodName,
                       GVariant* parameters,
-                      MethodResultBuilder& result) {
+                      MethodResultBuilder::Pointer result) {
         if (objectPath != TESTAPI_OBJECT_PATH || interface != TESTAPI_INTERFACE) {
             return;
         }
         if (methodName == TESTAPI_METHOD_NOOP) {
-            result.setVoid();
+            result->setVoid();
         } else if (methodName == TESTAPI_METHOD_PROCESS) {
             const gchar* arg = NULL;
             g_variant_get(parameters, "(&s)", &arg);
             std::string str = std::string("resp: ") + arg;
-            result.set(g_variant_new("(s)", str.c_str()));
+            result->set(g_variant_new("(s)", str.c_str()));
         } else if (methodName == TESTAPI_METHOD_THROW) {
             int arg = 0;
             g_variant_get(parameters, "(i)", &arg);
-            result.setError("org.tizen.containers.Error.Test", "msg: " + std::to_string(arg));
+            result->setError("org.tizen.containers.Error.Test", "msg: " + std::to_string(arg));
         }
     };
     conn1->registerObject(TESTAPI_OBJECT_PATH, TESTAPI_DEFINITION, handler);
@@ -450,6 +450,63 @@ BOOST_AUTO_TEST_CASE(MethodAsyncCallTest)
     BOOST_REQUIRE(callDone.wait(EVENT_TIMEOUT));
 }
 
+BOOST_AUTO_TEST_CASE(MethodAsyncCallAsyncHandlerTest)
+{
+    ScopedDbusDaemon daemon;
+    ScopedGlibLoop loop;
+    Latch nameAcquired;
+
+    DbusConnection::Pointer conn1 = DbusConnection::create(DBUS_ADDRESS);
+    DbusConnection::Pointer conn2 = DbusConnection::create(DBUS_ADDRESS);
+
+    conn1->setName(TESTAPI_BUS_NAME,
+                   [&] {nameAcquired.set();},
+                   [] {});
+    BOOST_REQUIRE(nameAcquired.wait(EVENT_TIMEOUT));
+
+    Latch handlerDone;
+    std::string strResult;
+    MethodResultBuilder::Pointer deferredResult;
+
+    auto handler = [&](const std::string& objectPath,
+                      const std::string& interface,
+                      const std::string& methodName,
+                      GVariant* parameters,
+                      MethodResultBuilder::Pointer result) {
+        if (objectPath != TESTAPI_OBJECT_PATH || interface != TESTAPI_INTERFACE) {
+            return;
+        }
+        if (methodName == TESTAPI_METHOD_PROCESS) {
+            const gchar* arg = NULL;
+            g_variant_get(parameters, "(&s)", &arg);
+            strResult = std::string("resp: ") + arg;
+            deferredResult = result;
+            handlerDone.set();
+        }
+    };
+    conn1->registerObject(TESTAPI_OBJECT_PATH, TESTAPI_DEFINITION, handler);
+
+    Latch callDone;
+
+    auto asyncResult = [&](dbus::AsyncMethodCallResult& asyncMethodCallResult) {
+        const gchar* ret = NULL;
+        g_variant_get(asyncMethodCallResult.get(), "(&s)", &ret);
+        BOOST_CHECK_EQUAL("resp: arg", ret);
+        callDone.set();
+    };
+    conn2->callMethodAsync(TESTAPI_BUS_NAME,
+                           TESTAPI_OBJECT_PATH,
+                           TESTAPI_INTERFACE,
+                           TESTAPI_METHOD_PROCESS,
+                           g_variant_new("(s)", "arg"),
+                           "(s)",
+                           asyncResult);
+    BOOST_REQUIRE(handlerDone.wait(EVENT_TIMEOUT));
+    BOOST_REQUIRE(callDone.empty());
+    deferredResult->set(g_variant_new("(s)", strResult.c_str()));
+    BOOST_REQUIRE(callDone.wait(EVENT_TIMEOUT));
+}
+
 BOOST_AUTO_TEST_CASE(MethodCallExceptionTest)
 {
     ScopedDbusDaemon daemon;