Add support for properties without an object manager
This allows for simpler client code when daemons expose a single object.
BUG=none
TEST=unit tests, verified property changed callbacks are received w/o object
manager.
Change-Id: I751b57175906071dbc9e8a8ddb56bdd269eee320
Reviewed-on: https://chromium-review.googlesource.com/370301
Commit-Ready: Christopher Book <cbook@chromium.org>
Tested-by: Christopher Book <cbook@chromium.org>
Reviewed-by: Christopher Book <cbook@chromium.org>
diff --git a/chromeos-dbus-bindings/proxy_generator.cc b/chromeos-dbus-bindings/proxy_generator.cc
index ea0d1dc..7f41eee 100644
--- a/chromeos-dbus-bindings/proxy_generator.cc
+++ b/chromeos-dbus-bindings/proxy_generator.cc
@@ -211,11 +211,15 @@
for (const auto& signal : interface.signals) {
AddSignalHandlerRegistration(signal, interface.name, true, text);
}
- AddProperties(config, interface, true, text);
+ AddProperties(interface, true, text);
text->AddBlankLine();
text->AddLine("virtual const dbus::ObjectPath& GetObjectPath() const = 0;");
- if (!config.object_manager.name.empty() && !interface.properties.empty())
- AddPropertyPublicMethods(proxy_name, true, text);
+ if (!interface.properties.empty()) {
+ if (config.object_manager.name.empty())
+ AddInitializeProperties(proxy_name, true, text);
+ else
+ AddSetPropertyChanged(proxy_name, true, text);
+ }
text->PopOffset();
text->AddLine("};");
@@ -252,13 +256,18 @@
AddReleaseObjectProxy(text);
AddGetObjectPath(text);
AddGetObjectProxy(text);
- if (!config.object_manager.name.empty() && !interface.properties.empty())
- AddPropertyPublicMethods(proxy_name, false, text);
+ if (!interface.properties.empty()) {
+ if (config.object_manager.name.empty())
+ AddInitializeProperties(proxy_name, false, text);
+ else
+ AddSetPropertyChanged(proxy_name, false, text);
+ AddGetProperties(text);
+ }
for (const auto& method : interface.methods) {
AddMethodProxy(method, interface.name, false, text);
AddAsyncMethodProxy(method, interface.name, false, text);
}
- AddProperties(config, interface, false, text);
+ AddProperties(interface, false, text);
text->PopOffset();
text->AddBlankLine();
@@ -288,6 +297,8 @@
proxy_name.c_str()));
}
text->AddLine("dbus::ObjectProxy* dbus_object_proxy_;");
+ if (config.object_manager.name.empty() && !interface.properties.empty())
+ text->AddLine("std::unique_ptr<PropertySet> property_set_;");
text->AddBlankLine();
if (!config.object_manager.name.empty() && !interface.properties.empty()) {
@@ -450,9 +461,36 @@
}
// static
-void ProxyGenerator::AddPropertyPublicMethods(const string& class_name,
- bool declaration_only,
- IndentedText* text) {
+void ProxyGenerator::AddInitializeProperties(const string& class_name,
+ bool declaration_only,
+ IndentedText* text) {
+ text->AddBlankLine();
+ text->AddLine(StringPrintf("%svoid InitializeProperties(",
+ declaration_only ? "virtual " : ""));
+ text->AddLineWithOffset(
+ StringPrintf("const base::Callback<void(%sInterface*, "
+ "const std::string&)>& callback) %s",
+ class_name.c_str(),
+ declaration_only ? "= 0;" : "override {"),
+ kLineContinuationOffset);
+ if (!declaration_only) {
+ IndentedText block;
+ block.PushOffset(kBlockOffset);
+ block.AddLine("property_set_.reset(");
+ block.AddLineWithOffset(
+ "new PropertySet(dbus_object_proxy_, base::Bind(callback, this)));",
+ kLineContinuationOffset);
+ block.AddLine("property_set_->ConnectSignals();");
+ block.AddLine("property_set_->GetAll();");
+ text->AddBlock(block);
+ text->AddLine("}");
+ }
+}
+
+// static
+void ProxyGenerator::AddSetPropertyChanged(const string& class_name,
+ bool declaration_only,
+ IndentedText* text) {
text->AddBlankLine();
text->AddLine(StringPrintf("%svoid SetPropertyChangedCallback(",
declaration_only ? "virtual " : ""));
@@ -465,15 +503,21 @@
if (!declaration_only) {
text->AddLineWithOffset("on_property_changed_ = callback;", kBlockOffset);
text->AddLine("}");
- text->AddBlankLine();
-
- text->AddLine(
- "const PropertySet* GetProperties() const { return property_set_; }");
- text->AddLine("PropertySet* GetProperties() { return property_set_; }");
}
}
// static
+void ProxyGenerator::AddGetProperties(IndentedText* text) {
+ text->AddBlankLine();
+ // Dereference and take the address since sometimes property_set_ is a raw
+ // pointer and sometimes it is a std::unique_ptr.
+ text->AddLine(
+ "const PropertySet* GetProperties() const { "
+ "return &(*property_set_); }");
+ text->AddLine("PropertySet* GetProperties() { return &(*property_set_); }");
+}
+
+// static
void ProxyGenerator::AddOnPropertyChanged(IndentedText* text) {
text->AddLine("void OnPropertyChanged(const std::string& property_name) {");
text->PushOffset(kBlockOffset);
@@ -522,8 +566,7 @@
void ProxyGenerator::AddPropertySet(const ServiceConfig& config,
const Interface& interface,
IndentedText* text) {
- // Must have ObjectManager in order for property system to work correctly.
- if (config.object_manager.name.empty())
+ if (config.object_manager.name.empty() && interface.properties.empty())
return;
IndentedText block;
@@ -573,14 +616,9 @@
}
// static
-void ProxyGenerator::AddProperties(const ServiceConfig& config,
- const Interface& interface,
+void ProxyGenerator::AddProperties(const Interface& interface,
bool declaration_only,
IndentedText* text) {
- // Must have ObjectManager in order for property system to work correctly.
- if (config.object_manager.name.empty())
- return;
-
if (declaration_only && !interface.properties.empty())
text->AddBlankLine();
diff --git a/chromeos-dbus-bindings/proxy_generator.h b/chromeos-dbus-bindings/proxy_generator.h
index 57af0e0..dca6e07 100644
--- a/chromeos-dbus-bindings/proxy_generator.h
+++ b/chromeos-dbus-bindings/proxy_generator.h
@@ -73,10 +73,18 @@
// Generates GetObjectProxy() method.
static void AddGetObjectProxy(IndentedText* text);
- // Generates SetPropertyChangedCallback/GetProperties() methods.
- static void AddPropertyPublicMethods(const std::string& class_name,
- bool declaration_only,
- IndentedText* text);
+ // Generates InitializeProperties() method and callback.
+ static void AddInitializeProperties(const std::string& class_name,
+ bool declaration_only,
+ IndentedText* text);
+
+ // Generates SetPropertyChanged() method and callback.
+ static void AddSetPropertyChanged(const std::string& class_name,
+ bool declaration_only,
+ IndentedText* text);
+
+ // Generates GetProperties() methods.
+ static void AddGetProperties(IndentedText* text);
// Generates OnPropertyChanged() method.
static void AddOnPropertyChanged(IndentedText* text);
@@ -93,8 +101,7 @@
IndentedText* text);
// Generates the property accessors.
- static void AddProperties(const ServiceConfig& config,
- const Interface& interface,
+ static void AddProperties(const Interface& interface,
bool declaration_only,
IndentedText* text);
diff --git a/chromeos-dbus-bindings/proxy_generator_unittest.cc b/chromeos-dbus-bindings/proxy_generator_unittest.cc
index 728e977..8e3c0f0 100644
--- a/chromeos-dbus-bindings/proxy_generator_unittest.cc
+++ b/chromeos-dbus-bindings/proxy_generator_unittest.cc
@@ -668,8 +668,8 @@
on_property_changed_ = callback;
}
- const PropertySet* GetProperties() const { return property_set_; }
- PropertySet* GetProperties() { return property_set_; }
+ const PropertySet* GetProperties() const { return &(*property_set_); }
+ PropertySet* GetProperties() { return &(*property_set_); }
const std::string& data() const override {
return property_set_->data.value();
@@ -952,6 +952,133 @@
} // namespace org
)literal_string";
+const char kExpectedContentWithProperties[] = R"literal_string(
+#include <memory>
+#include <string>
+#include <vector>
+
+#include <base/bind.h>
+#include <base/callback.h>
+#include <base/logging.h>
+#include <base/macros.h>
+#include <base/memory/ref_counted.h>
+#include <brillo/any.h>
+#include <brillo/dbus/dbus_method_invoker.h>
+#include <brillo/dbus/dbus_property.h>
+#include <brillo/dbus/dbus_signal_handler.h>
+#include <brillo/errors/error.h>
+#include <brillo/variant_dictionary.h>
+#include <dbus/bus.h>
+#include <dbus/message.h>
+#include <dbus/object_manager.h>
+#include <dbus/object_path.h>
+#include <dbus/object_proxy.h>
+
+namespace org {
+namespace chromium {
+
+// Abstract interface proxy for org::chromium::Test.
+class TestProxyInterface {
+ public:
+ virtual ~TestProxyInterface() = default;
+
+ static const char* DataName() { return "Data"; }
+ virtual const std::string& data() const = 0;
+ 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;
+
+ virtual void InitializeProperties(
+ const base::Callback<void(TestProxyInterface*, const std::string&)>& callback) = 0;
+};
+
+} // namespace chromium
+} // namespace org
+
+namespace org {
+namespace chromium {
+
+// Interface proxy for org::chromium::Test.
+class TestProxy final : public TestProxyInterface {
+ public:
+ class PropertySet : public dbus::PropertySet {
+ public:
+ PropertySet(dbus::ObjectProxy* object_proxy,
+ const PropertyChangedCallback& callback)
+ : dbus::PropertySet{object_proxy,
+ "org.chromium.Test",
+ 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);
+ };
+
+ TestProxy(const scoped_refptr<dbus::Bus>& bus) :
+ bus_{bus},
+ dbus_object_proxy_{
+ bus_->GetObjectProxy(service_name_, object_path_)} {
+ }
+
+ ~TestProxy() override {
+ }
+
+ void ReleaseObjectProxy(const base::Closure& callback) {
+ bus_->RemoveObjectProxy(service_name_, object_path_, callback);
+ }
+
+ const dbus::ObjectPath& GetObjectPath() const override {
+ return object_path_;
+ }
+
+ dbus::ObjectProxy* GetObjectProxy() const { return dbus_object_proxy_; }
+
+ void InitializeProperties(
+ const base::Callback<void(TestProxyInterface*, const std::string&)>& callback) override {
+ property_set_.reset(
+ new PropertySet(dbus_object_proxy_, base::Bind(callback, this)));
+ property_set_->ConnectSignals();
+ property_set_->GetAll();
+ }
+
+ const PropertySet* GetProperties() const { return &(*property_set_); }
+ PropertySet* GetProperties() { return &(*property_set_); }
+
+ const std::string& data() const override {
+ return property_set_->data.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_->name.Set(value, callback);
+ }
+
+ private:
+ scoped_refptr<dbus::Bus> bus_;
+ const std::string service_name_{"org.chromium.Test"};
+ const dbus::ObjectPath object_path_{"/org/chromium/Test"};
+ dbus::ObjectProxy* dbus_object_proxy_;
+ std::unique_ptr<PropertySet> property_set_;
+
+ DISALLOW_COPY_AND_ASSIGN(TestProxy);
+};
+
+} // namespace chromium
+} // namespace org
+)literal_string";
+
const char kExpectedContentWithObjectManagerAndServiceName[] = R"literal_string(
#include <memory>
#include <string>
@@ -1375,6 +1502,24 @@
test_utils::EXPECT_TEXT_CONTAINED(kExpectedContentWithService, contents);
}
+TEST_F(ProxyGeneratorTest, GenerateAdaptorsWithProperties) {
+ Interface interface;
+ interface.name = "org.chromium.Test";
+ interface.path = "/org/chromium/Test";
+ interface.properties.emplace_back("Data", "s", "read");
+ interface.properties.emplace_back("Name", "s", "readwrite");
+ vector<Interface> interfaces{interface};
+ base::FilePath output_path = temp_dir_.path().Append("output2.h");
+ ServiceConfig config;
+ config.service_name = "org.chromium.Test";
+ EXPECT_TRUE(ProxyGenerator::GenerateProxies(config, interfaces, output_path));
+ string contents;
+ EXPECT_TRUE(base::ReadFileToString(output_path, &contents));
+ // The header guards contain the (temporary) filename, so we search for
+ // the content we need within the string.
+ test_utils::EXPECT_TEXT_CONTAINED(kExpectedContentWithProperties, contents);
+}
+
TEST_F(ProxyGeneratorTest, GenerateAdaptorsWithObjectManager) {
Interface interface;
interface.name = "org.chromium.Itf1";
