chromeos-dbus-bindings: Generator must expose property name accessors

D-Bus property names tend to be used in client code literally to
validate the property name in property change notifications, etc.

This forces clients to hard-code the name in their code and when
an interface definition changes in XML and new adaptor/proxies are
generated, the client code is not updated but still compiles, so the
breaking change is hard to notice.

Expose property name accessors in the generated code which makes the
client break at compile time if the property name is changed.

BUG=brillo:348
TEST=`FEATURES=test emerge-link chromeos-dbus-bindings`

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

4 files changed, 20 insertions, 7 deletions

diff --git a/chromeos-dbus-bindings/adaptor_generator.cc b/chromeos-dbus-bindings/adaptor_generator.cc
index ad5af85..97c4a8b 100644
--- a/chromeos-dbus-bindings/adaptor_generator.cc
+++ b/chromeos-dbus-bindings/adaptor_generator.cc
@@ -228,7 +228,7 @@ void AdaptorGenerator::RegisterInterface(const string& itf_name,
       text->PopOffset();
       text->PopOffset();
     }
-    text->AddLine(StringPrintf("itf->AddProperty(\"%s\", &%s_);",
+    text->AddLine(StringPrintf("itf->AddProperty(%sName(), &%s_);",
                                property.name.c_str(), variable_name.c_str()));
   }
 }
@@ -424,8 +424,12 @@ void AdaptorGenerator::AddPropertyMethodImplementation(
     CHECK(signature.Parse(property.type, &type));
     string variable_name = NameParser{property.name}.MakeVariableName();
 
-    // Getter method.
+    // Property name accessor.
     block.AddComments(property.doc_string);
+    block.AddLine(StringPrintf("static const char* %sName() { return \"%s\"; }",
+                               property.name.c_str(), property.name.c_str()));
+
+    // Getter method.
     block.AddLine(StringPrintf("%s Get%s() const {",
                                type.c_str(),
                                property.name.c_str()));
diff --git a/chromeos-dbus-bindings/adaptor_generator_unittest.cc b/chromeos-dbus-bindings/adaptor_generator_unittest.cc
index 7d3e59f..772552d 100644
--- a/chromeos-dbus-bindings/adaptor_generator_unittest.cc
+++ b/chromeos-dbus-bindings/adaptor_generator_unittest.cc
@@ -102,13 +102,13 @@ class TestAdaptor {
     signal_Update_ = itf->RegisterSignalOfType<SignalUpdateType>("Update");
     signal_Mapping_ = itf->RegisterSignalOfType<SignalMappingType>("Mapping");
 
-    itf->AddProperty("CharacterName", &character_name_);
+    itf->AddProperty(CharacterNameName(), &character_name_);
     write_property_.SetAccessMode(
         chromeos::dbus_utils::ExportedPropertyBase::Access::kReadWrite);
     write_property_.SetValidator(
         base::Bind(&TestAdaptor::ValidateWriteProperty,
                    base::Unretained(this)));
-    itf->AddProperty("WriteProperty", &write_property_);
+    itf->AddProperty(WritePropertyName(), &write_property_);
   }
 
   void SendUpdateSignal() {
@@ -124,6 +124,7 @@ class TestAdaptor {
       signal->Send(in_key, in_2);
   }
 
+  static const char* CharacterNameName() { return "CharacterName"; }
   std::string GetCharacterName() const {
     return character_name_.GetValue().Get<std::string>();
   }
@@ -131,6 +132,7 @@ class TestAdaptor {
     character_name_.SetValue(character_name);
   }
 
+  static const char* WritePropertyName() { return "WriteProperty"; }
   std::string GetWriteProperty() const {
     return write_property_.GetValue().Get<std::string>();
   }
diff --git a/chromeos-dbus-bindings/proxy_generator.cc b/chromeos-dbus-bindings/proxy_generator.cc
index adc7004..bdf21d5 100644
--- a/chromeos-dbus-bindings/proxy_generator.cc
+++ b/chromeos-dbus-bindings/proxy_generator.cc
@@ -522,7 +522,7 @@ void ProxyGenerator::AddPropertySet(const ServiceConfig& config,
   block.PushOffset(kBlockOffset);
   for (const auto& prop : interface.properties) {
     block.AddLine(
-        StringPrintf("RegisterProperty(\"%s\", &%s);",
+        StringPrintf("RegisterProperty(%sName(), &%s);",
                      prop.name.c_str(),
                      NameParser{prop.name}.MakeVariableName().c_str()));
   }
@@ -562,6 +562,12 @@ void ProxyGenerator::AddProperties(const ServiceConfig& config,
 
   DbusSignature signature;
   for (const auto& prop : interface.properties) {
+    if (declaration_only) {
+      text->AddLine(
+          StringPrintf("static const char* %sName() { return \"%s\"; }",
+                       prop.name.c_str(),
+                       prop.name.c_str()));
+    }
     string type;
     CHECK(signature.Parse(prop.type, &type));
     MakeConstReferenceIfNeeded(&type);
diff --git a/chromeos-dbus-bindings/proxy_generator_unittest.cc b/chromeos-dbus-bindings/proxy_generator_unittest.cc
index 1c04fbe..f0615a9 100644
--- a/chromeos-dbus-bindings/proxy_generator_unittest.cc
+++ b/chromeos-dbus-bindings/proxy_generator_unittest.cc
@@ -564,6 +564,7 @@ namespace chromium {
 // Abstract interface proxy for org::chromium::Itf1.
 class Itf1ProxyInterface {
  public:
+  static const char* DataName() { return "Data"; }
   virtual const std::string& data() const = 0;
 
  protected:
@@ -586,7 +587,7 @@ class Itf1Proxy final : public Itf1ProxyInterface {
         : dbus::PropertySet{object_proxy,
                             "org.chromium.Itf1",
                             callback} {
-      RegisterProperty("data", &data);
+      RegisterProperty(DataName(), &data);
     }
 
     chromeos::dbus_utils::Property<std::string> data;
@@ -1338,7 +1339,7 @@ TEST_F(ProxyGeneratorTest, GenerateAdaptorsWithObjectManager) {
   interface.name = "org.chromium.Itf1";
   interface.path = "/org/chromium/Test/Object";
   interface.signals.emplace_back("Closer");
-  interface.properties.emplace_back("data", "s", "read");
+  interface.properties.emplace_back("Data", "s", "read");
   Interface interface2;
   interface2.name = "org.chromium.Itf2";
   vector<Interface> interfaces{interface, interface2};