[Floss] Refactor FlossAdapterClient with generalized method call

FlossAdapterClient's D-Bus method calls contain mostly boilerplate
codes; this refactors the common logic to a reusable utility function so
that each method call can only contain just a minimum definition without
code repetitions.

This will pay off in the next patches where we add a lot more interfaces
that map to D-Bus method calls.

Bug: b:202334519
Change-Id: I6da47c3427a296f1be98946bf638c248dbf2026e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3254976
Commit-Queue: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: Chris Mumford <cmumford@google.com>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/main@{#939086}

device/bluetooth/floss/floss_adapter_client.cc

@@ -39,57 +39,18 @@ constexpr char FlossAdapterClient::kExportedCallbacksPath[] =
     "/org/chromium/bluetooth/adapterclient";
 
 void FlossAdapterClient::StartDiscovery(ResponseCallback<Void> callback) {
-  dbus::ObjectProxy* object_proxy =
-      bus_->GetObjectProxy(service_name_, adapter_path_);
-  if (!object_proxy) {
-    std::move(callback).Run(absl::nullopt,
-                            Error(kErrorUnknownAdapter, std::string()));
-    return;
-  }
-
-  dbus::MethodCall method_call(kAdapterInterface, adapter::kStartDiscovery);
-  object_proxy->CallMethodWithErrorResponse(
-      &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
-      base::BindOnce(&FlossAdapterClient::DefaultResponseWithCallback<Void>,
-                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+  CallAdapterMethod0<Void>(std::move(callback), adapter::kStartDiscovery);
 }
 
 void FlossAdapterClient::CancelDiscovery(ResponseCallback<Void> callback) {
-  dbus::ObjectProxy* object_proxy =
-      bus_->GetObjectProxy(service_name_, adapter_path_);
-  if (!object_proxy) {
-    std::move(callback).Run(absl::nullopt,
-                            Error(kErrorUnknownAdapter, std::string()));
-    return;
-  }
-
-  dbus::MethodCall method_call(kAdapterInterface, adapter::kCancelDiscovery);
-  object_proxy->CallMethodWithErrorResponse(
-      &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
-      base::BindOnce(&FlossAdapterClient::DefaultResponseWithCallback<Void>,
-                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+  CallAdapterMethod0<Void>(std::move(callback), adapter::kCancelDiscovery);
 }
 
 void FlossAdapterClient::CreateBond(ResponseCallback<Void> callback,
                                     FlossDeviceId device,
                                     BluetoothTransport transport) {
-  dbus::ObjectProxy* object_proxy =
-      bus_->GetObjectProxy(service_name_, adapter_path_);
-  if (!object_proxy) {
-    std::move(callback).Run(absl::nullopt,
-                            Error(kErrorUnknownAdapter, std::string()));
-    return;
-  }
-
-  dbus::MethodCall method_call(kAdapterInterface, adapter::kCreateBond);
-  dbus::MessageWriter writer(&method_call);
-  SerializeFlossDeviceId(&writer, device);
-  writer.AppendUint32(static_cast<uint32_t>(transport));
-
-  object_proxy->CallMethodWithErrorResponse(
-      &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
-      base::BindOnce(&FlossAdapterClient::DefaultResponseWithCallback<Void>,
-                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+  CallAdapterMethod2<Void>(std::move(callback), adapter::kCreateBond, device,
+                           transport);
 }
 
 void FlossAdapterClient::Init(dbus::Bus* bus,
@@ -438,4 +399,101 @@ void FlossAdapterClient::SerializeFlossDeviceId(
   writer->CloseContainer(&array);
 }
 
+template <>
+void FlossAdapterClient::WriteDBusParam(dbus::MessageWriter* writer,
+                                        const FlossDeviceId& data) {
+  SerializeFlossDeviceId(writer, data);
+}
+
+template <>
+void FlossAdapterClient::WriteDBusParam(dbus::MessageWriter* writer,
+                                        const BluetoothTransport& data) {
+  writer->AppendUint32(static_cast<uint32_t>(data));
+}
+
+template <>
+void FlossAdapterClient::WriteDBusParam(dbus::MessageWriter* writer,
+                                        const uint32_t& data) {
+  writer->AppendUint32(data);
+}
+
+template <>
+void FlossAdapterClient::WriteDBusParam(dbus::MessageWriter* writer,
+                                        const std::string& data) {
+  writer->AppendString(data);
+}
+
+template <typename R, typename F>
+void FlossAdapterClient::CallAdapterMethod(ResponseCallback<R> callback,
+                                           const char* member,
+                                           F write_data) {
+  dbus::ObjectProxy* object_proxy =
+      bus_->GetObjectProxy(service_name_, adapter_path_);
+  if (!object_proxy) {
+    LOG(ERROR) << "Adapter proxy does not exist when trying to call " << member;
+    std::move(callback).Run(absl::nullopt,
+                            Error(kErrorUnknownAdapter, std::string()));
+    return;
+  }
+
+  dbus::MethodCall method_call(kAdapterInterface, member);
+  dbus::MessageWriter writer(&method_call);
+
+  write_data(&writer);
+
+  object_proxy->CallMethodWithErrorResponse(
+      &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+      base::BindOnce(&FlossAdapterClient::DefaultResponseWithCallback<R>,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+}
+
+template <typename R>
+void FlossAdapterClient::CallAdapterMethod0(ResponseCallback<R> callback,
+                                            const char* member) {
+  CallAdapterMethod(std::move(callback), member,
+                    [](dbus::MessageWriter* writer) {});
+}
+
+template <typename R, typename T1>
+void FlossAdapterClient::CallAdapterMethod1(ResponseCallback<R> callback,
+                                            const char* member,
+                                            const T1& arg1) {
+  CallAdapterMethod(std::move(callback), member,
+                    [&arg1](dbus::MessageWriter* writer) {
+                      FlossAdapterClient::WriteDBusParam(writer, arg1);
+                    });
+}
+
+template <typename R, typename T1, typename T2>
+void FlossAdapterClient::CallAdapterMethod2(ResponseCallback<R> callback,
+                                            const char* member,
+                                            const T1& arg1,
+                                            const T2& arg2) {
+  CallAdapterMethod(std::move(callback), member,
+                    [&arg1, &arg2](dbus::MessageWriter* writer) {
+                      FlossAdapterClient::WriteDBusParam(writer, arg1);
+                      FlossAdapterClient::WriteDBusParam(writer, arg2);
+                    });
+}
+
+// These methods are explicitly instantiated for FlossAdapterClientTest since
+// now we don't have many methods that cover the various template use cases.
+// TODO(b/202334519): Remove these and replace with real methods that implicitly
+// instantiate the template once we have more coverage.
+template void FlossAdapterClient::CallAdapterMethod0(
+    ResponseCallback<Void> callback,
+    const char* member);
+template void FlossAdapterClient::CallAdapterMethod0(
+    ResponseCallback<uint8_t> callback,
+    const char* member);
+template void FlossAdapterClient::CallAdapterMethod1(
+    ResponseCallback<std::string> callback,
+    const char* member,
+    const uint32_t& arg1);
+template void FlossAdapterClient::CallAdapterMethod2(
+    ResponseCallback<Void> callback,
+    const char* member,
+    const uint32_t& arg1,
+    const std::string& arg2);
+
 }  // namespace floss

device/bluetooth/floss/floss_adapter_client.h

@@ -8,6 +8,7 @@
 #include <string>
 
 #include "base/callback.h"
+#include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
@@ -190,6 +191,30 @@ class DEVICE_BLUETOOTH_EXPORT FlossAdapterClient : public FlossDBusClient {
   std::string adapter_address_;
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(FlossAdapterClientTest, CallAdapterMethods);
+
+  template <typename T>
+  static void WriteDBusParam(dbus::MessageWriter* writer, const T& data);
+
+  template <typename R, typename F>
+  void CallAdapterMethod(ResponseCallback<R> callback,
+                         const char* member,
+                         F write_data);
+
+  template <typename R>
+  void CallAdapterMethod0(ResponseCallback<R> callback, const char* member);
+
+  template <typename R, typename T1>
+  void CallAdapterMethod1(ResponseCallback<R> callback,
+                          const char* member,
+                          const T1& arg1);
+
+  template <typename R, typename T1, typename T2>
+  void CallAdapterMethod2(ResponseCallback<R> callback,
+                          const char* member,
+                          const T1& arg1,
+                          const T2& arg2);
+
   // Object path for exported callbacks registered against adapter interface.
   static const char kExportedCallbacksPath[];
 

device/bluetooth/floss/floss_adapter_client_unittest.cc

@@ -31,6 +31,15 @@ MATCHER_P(HasMemberOf, member, "") {
   return arg->GetMember() == member;
 }
 
+constexpr char kTestMethod0[] = "TestMethod0";
+constexpr char kTestMethod1[] = "TestMethod1";
+constexpr char kTestMethod2[] = "TestMethod2";
+
+constexpr uint8_t kFakeU8Return = 100;
+constexpr char kFakeStrReturn[] = "fake return";
+constexpr uint32_t kFakeU32Param = 30;
+constexpr char kFakeStrParam[] = "fake param";
+
 }  // namespace
 
 namespace floss {
@@ -450,4 +459,133 @@ TEST_F(FlossAdapterClientTest, CreateBond) {
       bond, transport);
 }
 
+TEST_F(FlossAdapterClientTest, CallAdapterMethods) {
+  client_->Init(bus_.get(), kAdapterInterface, adapter_path_.value());
+
+  // Method of 0 parameters with no return.
+  EXPECT_CALL(*adapter_object_proxy_.get(),
+              DoCallMethodWithErrorResponse(HasMemberOf(kTestMethod0), _, _))
+      .WillOnce([](::dbus::MethodCall* method_call, int timeout_ms,
+                   ::dbus::ObjectProxy::ResponseOrErrorCallback* cb) {
+        dbus::MessageReader msg(method_call);
+        // D-Bus method call should have no parameters.
+        EXPECT_FALSE(msg.HasMoreData());
+        // Create a fake response with no return value.
+        auto response = ::dbus::Response::CreateEmpty();
+        std::move(*cb).Run(response.get(), nullptr);
+      });
+  client_->CallAdapterMethod0(
+      base::BindOnce([](const absl::optional<Void>& ret,
+                        const absl::optional<Error>& err) {
+        // Check that there should be no return and error.
+        EXPECT_FALSE(err.has_value());
+        EXPECT_FALSE(ret.has_value());
+      }),
+      kTestMethod0);
+
+  // Method of 0 parameters with uint8_t return.
+  EXPECT_CALL(*adapter_object_proxy_.get(),
+              DoCallMethodWithErrorResponse(HasMemberOf(kTestMethod0), _, _))
+      .WillOnce([](::dbus::MethodCall* method_call, int timeout_ms,
+                   ::dbus::ObjectProxy::ResponseOrErrorCallback* cb) {
+        dbus::MessageReader msg(method_call);
+        // D-Bus method call should have no parameters.
+        EXPECT_FALSE(msg.HasMoreData());
+        // Create a fake response with a return value.
+        auto response = ::dbus::Response::CreateEmpty();
+        dbus::MessageWriter writer(response.get());
+        writer.AppendByte(kFakeU8Return);
+        std::move(*cb).Run(response.get(), nullptr);
+      });
+  client_->CallAdapterMethod0(
+      base::BindOnce([](const absl::optional<uint8_t>& ret,
+                        const absl::optional<Error>& err) {
+        // Check that return is correctly parsed and there should be no error.
+        EXPECT_FALSE(err.has_value());
+        EXPECT_TRUE(ret.has_value());
+        EXPECT_EQ(100, ret);
+      }),
+      kTestMethod0);
+
+  // Method of 1 parameter with string return.
+  EXPECT_CALL(*adapter_object_proxy_.get(),
+              DoCallMethodWithErrorResponse(HasMemberOf(kTestMethod1), _, _))
+      .WillOnce([](::dbus::MethodCall* method_call, int timeout_ms,
+                   ::dbus::ObjectProxy::ResponseOrErrorCallback* cb) {
+        dbus::MessageReader msg(method_call);
+        // D-Bus method call should have 1 parameter.
+        uint32_t param1;
+        ASSERT_TRUE(msg.PopUint32(&param1));
+        EXPECT_EQ(kFakeU32Param, param1);
+        EXPECT_FALSE(msg.HasMoreData());
+        // Create a fake response with a return value.
+        auto response = ::dbus::Response::CreateEmpty();
+        dbus::MessageWriter writer(response.get());
+        writer.AppendString(kFakeStrReturn);
+        std::move(*cb).Run(response.get(), nullptr);
+      });
+  client_->CallAdapterMethod1(
+      base::BindOnce([](const absl::optional<std::string>& ret,
+                        const absl::optional<Error>& err) {
+        // Check that return is correctly parsed and there should be no error.
+        EXPECT_FALSE(err.has_value());
+        EXPECT_TRUE(ret.has_value());
+        EXPECT_EQ(kFakeStrReturn, ret);
+      }),
+      kTestMethod1, kFakeU32Param);
+
+  // Method of 2 parameters with no return.
+  EXPECT_CALL(*adapter_object_proxy_.get(),
+              DoCallMethodWithErrorResponse(HasMemberOf(kTestMethod2), _, _))
+      .WillOnce([](::dbus::MethodCall* method_call, int timeout_ms,
+                   ::dbus::ObjectProxy::ResponseOrErrorCallback* cb) {
+        dbus::MessageReader msg(method_call);
+        // D-Bus method call should have 2 parameter.
+        uint32_t param1;
+        std::string param2;
+        ASSERT_TRUE(msg.PopUint32(&param1));
+        ASSERT_TRUE(msg.PopString(&param2));
+        EXPECT_EQ(kFakeU32Param, param1);
+        EXPECT_EQ(kFakeStrParam, param2);
+        EXPECT_FALSE(msg.HasMoreData());
+        // Create a fake response with no return value.
+        auto response = ::dbus::Response::CreateEmpty();
+        std::move(*cb).Run(response.get(), nullptr);
+      });
+  std::string str_param(kFakeStrParam);
+  client_->CallAdapterMethod2(
+      base::BindOnce([](const absl::optional<Void>& ret,
+                        const absl::optional<Error>& err) {
+        // Check that there should be no return and error.
+        EXPECT_FALSE(err.has_value());
+        EXPECT_FALSE(ret.has_value());
+      }),
+      kTestMethod2, kFakeU32Param, str_param);
+
+  // Method of 0 parameters with invalid return.
+  EXPECT_CALL(*adapter_object_proxy_.get(),
+              DoCallMethodWithErrorResponse(HasMemberOf(kTestMethod0), _, _))
+      .WillOnce([](::dbus::MethodCall* method_call, int timeout_ms,
+                   ::dbus::ObjectProxy::ResponseOrErrorCallback* cb) {
+        dbus::MessageReader msg(method_call);
+        // D-Bus method call should have no parameters.
+        EXPECT_FALSE(msg.HasMoreData());
+        // Create a fake response with a return value.
+        auto response = ::dbus::Response::CreateEmpty();
+        dbus::MessageWriter writer(response.get());
+        writer.AppendUint32(kFakeU8Return);
+        std::move(*cb).Run(response.get(), nullptr);
+      });
+  client_->CallAdapterMethod0(
+      base::BindOnce([](const absl::optional<uint8_t>& ret,
+                        const absl::optional<Error>& err) {
+        // Check that return cannot be parsed and there should be an error.
+        EXPECT_TRUE(err.has_value());
+        EXPECT_FALSE(ret.has_value());
+        EXPECT_EQ(FlossDBusClient::kErrorInvalidReturn, err->name);
+        EXPECT_EQ(std::string(), err->message);
+      }),
+      kTestMethod0);
+}
+
 }  // namespace floss