peerd: Use D-Bus codegen to create correct handler for ExposeService

Use the new annotation to mark ExposeService as a handler that takes
the raw D-Bus message which it can then use to extract the sender's
unique name.

BUG=brillo:346
TEST=`FEATURES=test emerge-link peerd`

Change-Id: I176864f347eae7b39201bf23f1eadbcc7bbd52f5
Reviewed-on: https://chromium-review.googlesource.com/252322
Tested-by: Alex Vakulenko <avakulenko@chromium.org>
Reviewed-by: Christopher Wiley <wiley@chromium.org>
Commit-Queue: Alex Vakulenko <avakulenko@chromium.org>

4 files changed, 25 insertions, 64 deletions

diff --git a/dbus_bindings/org.chromium.peerd.Manager.xml b/dbus_bindings/org.chromium.peerd.Manager.xml
index 67dab85..97376eb 100644
--- a/dbus_bindings/org.chromium.peerd.Manager.xml
+++ b/dbus_bindings/org.chromium.peerd.Manager.xml
@@ -23,7 +23,9 @@
       <arg name="service_info" type="a{ss}" direction="in"/>
       <arg name="options" type="a{sv}" direction="in"/>
       <arg name="service_token" type="s" direction="out"/>
-      <annotation name="org.chromium.DBus.Method.Kind" value="raw"/>
+      <annotation name="org.chromium.DBus.Method.Kind" value="normal"/>
+      <annotation name="org.chromium.DBus.Method.IncludeDBusMessage"
+                  value="true"/>
     </method>
     <method name="RemoveExposedService">
       <arg name="service_token" type="s" direction="in"/>
diff --git a/manager.cc b/manager.cc
index bf6ea88..b0f1e74 100644
--- a/manager.cc
+++ b/manager.cc
@@ -39,41 +39,6 @@ using std::unique_ptr;
 using std::vector;
 
 namespace peerd {
-
-namespace {
-
-template<typename... Args>
-void RedirectDbusCall(Manager* mgr,
-                      bool (Manager::*fnc)(chromeos::ErrorPtr* /*error*/,
-                                           const std::string& /*sender_addr*/,
-                                           Args...),
-                      dbus::MethodCall* method_call,
-                      chromeos::dbus_utils::ResponseSender sender) {
-  chromeos::dbus_utils::DBusMethodResponseBase method_response(method_call,
-                                                               sender);
-  auto invoke_callback =
-      [mgr, fnc, method_call, &method_response](const Args&... args) {
-    ErrorPtr error;
-    if (!(mgr->*fnc)(&error, method_call->GetSender(), args...)) {
-      method_response.ReplyWithError(error.get());
-    } else {
-      auto response = method_response.CreateCustomResponse();
-      dbus::MessageWriter writer(response.get());
-      DBusParamWriter::AppendDBusOutParams(&writer, args...);
-      method_response.SendRawResponse(response.Pass());
-    }
-  };
-  ErrorPtr param_reader_error;
-  dbus::MessageReader reader(method_call);
-  if (!DBusParamReader<true, Args...>::Invoke(invoke_callback, &reader,
-                                              &param_reader_error)) {
-    // Error parsing method arguments.
-    method_response.ReplyWithError(param_reader_error.get());
-  }
-}
-
-}  // namespace
-
 namespace errors {
 namespace manager {
 
@@ -214,17 +179,12 @@ bool Manager::StopMonitoring(chromeos::ErrorPtr* error,
 }
 
 
-void Manager::ExposeService(dbus::MethodCall* method_call,
-                            chromeos::dbus_utils::ResponseSender sender) {
-  RedirectDbusCall(this, &Manager::ExposeServiceImpl, method_call, sender);
-}
-
-bool Manager::ExposeServiceImpl(chromeos::ErrorPtr* error,
-                                const std::string& sender,
-                                const string& service_id,
-                                const map<string, string>& service_info,
-                                const map<string, Any>& options,
-                                std::string* service_token) {
+bool Manager::ExposeService(chromeos::ErrorPtr* error,
+                            dbus::Message* message,
+                            const string& service_id,
+                            const map<string, string>& service_info,
+                            const map<string, Any>& options,
+                            std::string* service_token) {
   VLOG(1) << "Exposing service '" << service_id << "'.";
   if (service_id == kSerbusServiceId) {
     Error::AddToPrintf(error,
@@ -242,7 +202,7 @@ bool Manager::ExposeServiceImpl(chromeos::ErrorPtr* error,
   // TODO(wiley) Maybe trigger an advertisement run since we have updated
   //             information.
   std::unique_ptr<DBusServiceWatcher> service_watcher{
-      new DBusServiceWatcher{bus_, sender,
+      new DBusServiceWatcher{bus_, message->GetSender(),
                              base::Bind(&Manager::OnDBusServiceDeath,
                                         base::Unretained(this),
                                         *service_token)}};
diff --git a/manager.h b/manager.h
index ccc2514..f634f72 100644
--- a/manager.h
+++ b/manager.h
@@ -63,8 +63,13 @@ class Manager : public org::chromium::peerd::ManagerInterface {
       const std::map<std::string, chromeos::Any>& options,
       std::string* monitoring_token) override;
 
-  void ExposeService(dbus::MethodCall* method_call,
-                     chromeos::dbus_utils::ResponseSender sender) override;
+  bool ExposeService(
+      chromeos::ErrorPtr* error,
+      dbus::Message* message,
+      const std::string& service_id,
+      const std::map<std::string, std::string>& service_info,
+      const std::map<std::string, chromeos::Any>& options,
+      std::string* service_token) override;
 
   bool StopMonitoring(
       chromeos::ErrorPtr* error,
@@ -85,15 +90,6 @@ class Manager : public org::chromium::peerd::ManagerInterface {
           std::unique_ptr<AvahiClient> avahi_client,
           const std::string& initial_mdns_prefix);
 
-  bool ExposeServiceImpl(
-      chromeos::ErrorPtr* error,
-      const std::string& sender,
-      const std::string& service_id,
-      const std::map<std::string, std::string>& service_info,
-      const std::map<std::string, chromeos::Any>& options,
-      std::string* service_token);
-
-
   // Called from AvahiClient.
   void ShouldRefreshAvahiPublisher();
 
diff --git a/manager_unittest.cc b/manager_unittest.cc
index 4de664d..9a0657d 100644
--- a/manager_unittest.cc
+++ b/manager_unittest.cc
@@ -91,8 +91,9 @@ class ManagerTest : public testing::Test {
 TEST_F(ManagerTest, ShouldRejectSerbusServiceId) {
   chromeos::ErrorPtr error;
   string service_token;
-  EXPECT_FALSE(manager_->ExposeServiceImpl(&error, "", kSerbusServiceId, {}, {},
-               &service_token));
+  dbus::MethodCall method_call{"org.chromium.peerd.Manager", "ExposeService"};
+  EXPECT_FALSE(manager_->ExposeService(&error, &method_call, kSerbusServiceId,
+                                       {}, {}, &service_token));
   EXPECT_TRUE(service_token.empty());
   EXPECT_NE(nullptr, error.get());
 }
@@ -172,14 +173,16 @@ TEST_F(ManagerTest, StopMonitoring_HandlesMultipleSubscriptions) {
 
 TEST_F(ManagerTest, ShouldRemoveServicesOnRemoteDeath) {
   const string kServiceId{"a_service_id"};
-  const string kDBusSender{"a-dbus-connection-id"};
+  const string kDBusSender{"a.dbus.connection.id"};
   EXPECT_CALL(*peer_, AddPublishedService(_, kServiceId, _, _))
       .WillOnce(Return(true));
   EXPECT_CALL(*mock_bus_, ListenForServiceOwnerChange(kDBusSender, _));
   EXPECT_CALL(*mock_bus_, GetServiceOwner(kDBusSender, _));
   string service_token;
-  EXPECT_TRUE(manager_->ExposeServiceImpl(
-        nullptr, kDBusSender, kServiceId, {}, {}, &service_token));
+  dbus::MethodCall method_call{"org.chromium.peerd.Manager", "ExposeService"};
+  method_call.SetSender(kDBusSender);
+  EXPECT_TRUE(manager_->ExposeService(nullptr, &method_call, kServiceId, {}, {},
+                                      &service_token));
   Mock::VerifyAndClearExpectations(peer_);
   Mock::VerifyAndClearExpectations(mock_bus_.get());
   EXPECT_CALL(*peer_, RemoveService(_, kServiceId));