dbus-binding-generator: Fix mock generation and support for readwrite props

Previous change added "set_<name>()" for writable properties, but that
was done unconditionally, even for read-only properties. Added check for
property access qualifier.

Mock generation was missing for GetObjectPath and property setters.

BUG: 26092352
Change-Id: Ib30267f41c7a937ea8543f02bbe17719ef15cca9

3 files changed, 45 insertions, 20 deletions

diff --git a/chromeos-dbus-bindings/proxy_generator.cc b/chromeos-dbus-bindings/proxy_generator.cc
index 5a18bc1..8bcc5cb 100644
--- a/chromeos-dbus-bindings/proxy_generator.cc
+++ b/chromeos-dbus-bindings/proxy_generator.cc
@@ -341,7 +341,14 @@ void ProxyGenerator::GenerateInterfaceMock(const ServiceConfig& config,
     string name = NameParser{prop.name}.MakeVariableName();
     text->AddLine(StringPrintf("MOCK_CONST_METHOD0(%s, %s());",
                                name.c_str(), type.c_str()));
+    if (prop.access == "readwrite") {
+      text->AddLine(StringPrintf("MOCK_METHOD2(set_%s, void(%s, "
+                                 "const base::Callback<bool>&));",
+                                 name.c_str(), type.c_str()));
+    }
   }
+  text->AddLine(
+      "MOCK_CONST_METHOD0(GetObjectPath, const dbus::ObjectPath&());");
 
   text->PopOffset();
   text->AddBlankLine();
@@ -586,23 +593,25 @@ void ProxyGenerator::AddProperties(const ServiceConfig& config,
           kBlockOffset);
       text->AddLine("}");
     }
-    if (!declaration_only)
-      text->AddBlankLine();
-    text->AddLineAndPushOffsetTo(
-        StringPrintf("%svoid set_%s(%s value,",
-                     declaration_only ? "virtual " : "",
-                     name.c_str(),
-                     type.c_str()),
-        1, '(');
-    text->AddLine(
-        StringPrintf("const base::Callback<void(bool)>& callback)%s",
-                     declaration_only ? " = 0;" : " override {"));
-    text->PopOffset();
-    if (!declaration_only) {
-      text->AddLineWithOffset(
-          StringPrintf("property_set_->%s.Set(value, callback);", name.c_str()),
-          kBlockOffset);
-      text->AddLine("}");
+    if (prop.access == "readwrite") {
+      if (!declaration_only)
+        text->AddBlankLine();
+      text->AddLineAndPushOffsetTo(
+          StringPrintf("%svoid set_%s(%s value,",
+                       declaration_only ? "virtual " : "",
+                       name.c_str(),
+                       type.c_str()),
+          1, '(');
+      text->AddLine(
+          StringPrintf("const base::Callback<void(bool)>& callback)%s",
+                       declaration_only ? " = 0;" : " override {"));
+      text->PopOffset();
+      if (!declaration_only) {
+        text->AddLineWithOffset(
+            StringPrintf("property_set_->%s.Set(value, callback);", name.c_str()),
+            kBlockOffset);
+        text->AddLine("}");
+      }
     }
   }
 }
diff --git a/chromeos-dbus-bindings/proxy_generator_mock_unittest.cc b/chromeos-dbus-bindings/proxy_generator_mock_unittest.cc
index 76f0ee5..692ca84 100644
--- a/chromeos-dbus-bindings/proxy_generator_mock_unittest.cc
+++ b/chromeos-dbus-bindings/proxy_generator_mock_unittest.cc
@@ -123,6 +123,7 @@ class TestInterfaceProxyMock : public TestInterfaceProxyInterface {
                void(const base::Callback<void(const std::vector<std::string>&,
                                               uint8_t)>& /*signal_callback*/,
                     dbus::ObjectProxy::OnConnectedCallback /*on_connected_callback*/));
+  MOCK_CONST_METHOD0(GetObjectPath, const dbus::ObjectPath&());
 
  private:
   DISALLOW_COPY_AND_ASSIGN(TestInterfaceProxyMock);
@@ -147,6 +148,10 @@ class TestInterface2ProxyMock : public TestInterface2ProxyInterface {
                void(const base::Callback<void(const std::string& /*name*/, int32_t /*age*/)>& /*success_callback*/,
                     const base::Callback<void(brillo::Error*)>& /*error_callback*/,
                     int /*timeout_ms*/));
+  MOCK_CONST_METHOD0(data, const std::string&());
+  MOCK_CONST_METHOD0(name, const std::string&());
+  MOCK_METHOD2(set_name, void(const std::string&, const base::Callback<bool>&));
+  MOCK_CONST_METHOD0(GetObjectPath, const dbus::ObjectPath&());
 
  private:
   DISALLOW_COPY_AND_ASSIGN(TestInterface2ProxyMock);
@@ -224,6 +229,8 @@ TEST_F(ProxyGeneratorMockTest, GenerateMocks) {
       vector<Interface::Argument>{
           {"name", kDBusTypeString},
           {"age", kDBusTypeInt32}});
+  interface2.properties.emplace_back("Data", "s", "read");
+  interface2.properties.emplace_back("Name", "s", "readwrite");
   vector<Interface> interfaces{interface, interface2};
   base::FilePath output_path = temp_dir_.path().Append("output.h");
   base::FilePath proxy_path = temp_dir_.path().Append("proxies.h");
diff --git a/chromeos-dbus-bindings/proxy_generator_unittest.cc b/chromeos-dbus-bindings/proxy_generator_unittest.cc
index 5648bc9..63ac43b 100644
--- a/chromeos-dbus-bindings/proxy_generator_unittest.cc
+++ b/chromeos-dbus-bindings/proxy_generator_unittest.cc
@@ -590,7 +590,9 @@ class Itf1ProxyInterface {
 
   static const char* DataName() { return "Data"; }
   virtual const std::string& data() const = 0;
-  virtual void set_data(const std::string& value,
+  static const char* NameName() { return "Name"; }
+  virtual const std::string& name() const = 0;
+  virtual void set_name(const std::string& value,
                         const base::Callback<void(bool)>& callback) = 0;
 
   virtual const dbus::ObjectPath& GetObjectPath() const = 0;
@@ -613,9 +615,11 @@ class Itf1Proxy final : public Itf1ProxyInterface {
                             "org.chromium.Itf1",
                             callback} {
       RegisterProperty(DataName(), &data);
+      RegisterProperty(NameName(), &name);
     }
 
     brillo::dbus_utils::Property<std::string> data;
+    brillo::dbus_utils::Property<std::string> name;
 
    private:
     DISALLOW_COPY_AND_ASSIGN(PropertySet);
@@ -668,9 +672,13 @@ class Itf1Proxy final : public Itf1ProxyInterface {
     return property_set_->data.value();
   }
 
-  void set_data(const std::string& value,
+  const std::string& name() const override {
+    return property_set_->name.value();
+  }
+
+  void set_name(const std::string& value,
                 const base::Callback<void(bool)>& callback) override {
-    property_set_->data.Set(value, callback);
+    property_set_->name.Set(value, callback);
   }
 
  private:
@@ -1371,6 +1379,7 @@ TEST_F(ProxyGeneratorTest, GenerateAdaptorsWithObjectManager) {
   interface.path = "/org/chromium/Test/Object";
   interface.signals.emplace_back("Closer");
   interface.properties.emplace_back("Data", "s", "read");
+  interface.properties.emplace_back("Name", "s", "readwrite");
   Interface interface2;
   interface2.name = "org.chromium.Itf2";
   vector<Interface> interfaces{interface, interface2};