buffet: Make CommandInstance own its proxies.
This allows proxies which are not managed by command dispatchers,
for example, cloud proxies.
BUG=None
TEST=cros_workon_make buffet --test
Change-Id: Ia7f0fe97c20328acf8e7942a6ad71aabaade81e4
Reviewed-on: https://chromium-review.googlesource.com/228821
Tested-by: Anton Muhin <antonm@chromium.org>
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Anton Muhin <antonm@chromium.org>
diff --git a/buffet/commands/command_instance.cc b/buffet/commands/command_instance.cc
index 0caee20..c83b48a 100644
--- a/buffet/commands/command_instance.cc
+++ b/buffet/commands/command_instance.cc
@@ -32,6 +32,8 @@
: name_(name), category_(category), parameters_(parameters) {
}
+CommandInstance::~CommandInstance() = default;
+
std::shared_ptr<const PropValue> CommandInstance::FindParameter(
const std::string& name) const {
std::shared_ptr<const PropValue> value;
@@ -138,6 +140,10 @@
return instance;
}
+void CommandInstance::AddProxy(std::unique_ptr<CommandProxyInterface> proxy) {
+ proxies_.push_back(std::move(proxy));
+}
+
bool CommandInstance::SetProgress(int progress) {
if (progress < 0 || progress > 100)
return false;
diff --git a/buffet/commands/command_instance.h b/buffet/commands/command_instance.h
index e35fd7b..b324850 100644
--- a/buffet/commands/command_instance.h
+++ b/buffet/commands/command_instance.h
@@ -34,6 +34,7 @@
CommandInstance(const std::string& name,
const std::string& category,
const native_types::Object& parameters);
+ ~CommandInstance();
// Returns the full command ID.
const std::string& GetID() const { return id_; }
@@ -61,9 +62,7 @@
void SetID(const std::string& id) { id_ = id; }
// Adds a proxy for this command.
// The proxy object is not owned by this class.
- void AddProxy(CommandProxyInterface* proxy) { proxies_.push_back(proxy); }
- // Removes all the proxies for this command.
- void ClearProxies() { proxies_.clear(); }
+ void AddProxy(std::unique_ptr<CommandProxyInterface> proxy);
// Sets the pointer to queue this command is part of.
void SetCommandQueue(CommandQueue* queue) { queue_ = queue; }
@@ -115,11 +114,13 @@
// Current command execution progress.
int progress_ = 0;
// Command proxies for the command.
- std::vector<CommandProxyInterface*> proxies_;
+ std::vector<std::unique_ptr<CommandProxyInterface>> proxies_;
// Pointer to the command queue this command instance is added to.
// The queue owns the command instance, so it outlives this object.
CommandQueue* queue_ = nullptr;
+ friend class DBusCommandDispacherTest;
+ friend class DBusCommandProxyTest;
DISALLOW_COPY_AND_ASSIGN(CommandInstance);
};
diff --git a/buffet/commands/dbus_command_dispatcher.cc b/buffet/commands/dbus_command_dispatcher.cc
index d36a927..6751372 100644
--- a/buffet/commands/dbus_command_dispatcher.cc
+++ b/buffet/commands/dbus_command_dispatcher.cc
@@ -24,17 +24,10 @@
void DBusCommandDispacher::OnCommandAdded(CommandInstance* command_instance) {
auto proxy = CreateDBusCommandProxy(command_instance);
proxy->RegisterAsync(AsyncEventSequencer::GetDefaultCompletionAction());
- command_instance->AddProxy(proxy.get());
-
- auto pair = std::make_pair(command_instance, std::move(proxy));
- CHECK(command_map_.insert(std::move(pair)).second)
- << "The command instance is already in the dispatcher command map";
+ command_instance->AddProxy(std::move(proxy));
}
void DBusCommandDispacher::OnCommandRemoved(CommandInstance* command_instance) {
- command_instance->ClearProxies();
- CHECK_GT(command_map_.erase(command_instance), 0u)
- << "The command instance is not in the dispatcher command map";
}
std::unique_ptr<DBusCommandProxy> DBusCommandDispacher::CreateDBusCommandProxy(
diff --git a/buffet/commands/dbus_command_dispatcher.h b/buffet/commands/dbus_command_dispatcher.h
index ca546e7..35d258b 100644
--- a/buffet/commands/dbus_command_dispatcher.h
+++ b/buffet/commands/dbus_command_dispatcher.h
@@ -49,9 +49,6 @@
scoped_refptr<dbus::Bus> bus_;
base::WeakPtr<chromeos::dbus_utils::ExportedObjectManager> object_manager_;
int next_id_;
- // This is the map that tracks relationship between CommandInstance and
- // corresponding DBusCommandProxy objects.
- std::map<CommandInstance*, std::unique_ptr<DBusCommandProxy>> command_map_;
// Default constructor is used in special circumstances such as for testing.
DBusCommandDispacher() = default;
diff --git a/buffet/commands/dbus_command_dispatcher_unittest.cc b/buffet/commands/dbus_command_dispatcher_unittest.cc
index 9808363..314135f 100644
--- a/buffet/commands/dbus_command_dispatcher_unittest.cc
+++ b/buffet/commands/dbus_command_dispatcher_unittest.cc
@@ -108,9 +108,8 @@
}
DBusCommandProxy* FindProxy(CommandInstance* command_instance) {
- const auto& command_map = command_dispatcher_->command_map_;
- auto it = command_map.find(command_instance);
- return it != command_map.end() ? it->second.get() : nullptr;
+ CHECK_EQ(command_instance->proxies_.size(), 1);
+ return static_cast<DBusCommandProxy*>(command_instance->proxies_[0].get());
}
void FinishCommand(DBusCommandProxy* proxy) {
@@ -156,7 +155,6 @@
EXPECT_CALL(*mock_exported_object_manager_, SendSignal(_)).Times(2);
FinishCommand(command_proxy);
- EXPECT_EQ(nullptr, FindProxy(command_instance));
EXPECT_EQ(nullptr, command_queue_.Find(id));
}
@@ -190,7 +188,6 @@
EXPECT_CALL(*mock_exported_object_manager_, SendSignal(_)).Times(2);
FinishCommand(command_proxy);
- EXPECT_EQ(nullptr, FindProxy(command_instance));
EXPECT_EQ(nullptr, command_queue_.Find(id));
}
diff --git a/buffet/commands/dbus_command_proxy_unittest.cc b/buffet/commands/dbus_command_proxy_unittest.cc
index ab4e36c..0c1bbaa 100644
--- a/buffet/commands/dbus_command_proxy_unittest.cc
+++ b/buffet/commands/dbus_command_proxy_unittest.cc
@@ -91,37 +91,39 @@
EXPECT_CALL(*mock_exported_object_command_,
ExportMethod(_, _, _, _)).Times(AnyNumber());
- command_proxy_.reset(new DBusCommandProxy(nullptr, bus_,
- command_instance_.get(),
- cmd_path));
- command_instance_->AddProxy(command_proxy_.get());
- command_proxy_->RegisterAsync(
+ std::unique_ptr<CommandProxyInterface> command_proxy(
+ new DBusCommandProxy(nullptr, bus_, command_instance_.get(), cmd_path));
+ command_instance_->AddProxy(std::move(command_proxy));
+ GetCommandProxy()->RegisterAsync(
AsyncEventSequencer::GetDefaultCompletionAction());
}
void TearDown() override {
EXPECT_CALL(*mock_exported_object_command_, Unregister()).Times(1);
- command_instance_->ClearProxies();
- command_proxy_.reset();
command_instance_.reset();
dict_.Clear();
bus_ = nullptr;
}
+ DBusCommandProxy* GetCommandProxy() const {
+ CHECK_EQ(command_instance_->proxies_.size(), 1);
+ return static_cast<DBusCommandProxy*>(command_instance_->proxies_[0].get());
+ }
+
chromeos::dbus_utils::DBusObject* GetProxyDBusObject() {
- return &command_proxy_->dbus_object_;
+ return &GetCommandProxy()->dbus_object_;
}
std::string GetStatus() const {
- return command_proxy_->status_.value();
+ return GetCommandProxy()->status_.value();
}
int32_t GetProgress() const {
- return command_proxy_->progress_.value();
+ return GetCommandProxy()->progress_.value();
}
VariantDictionary GetParameters() const {
- return command_proxy_->parameters_.value();
+ return GetCommandProxy()->parameters_.value();
}
std::unique_ptr<dbus::Response> CallMethod(
@@ -168,7 +170,6 @@
return value;
}
- std::unique_ptr<DBusCommandProxy> command_proxy_;
std::unique_ptr<CommandInstance> command_instance_;
CommandDictionary dict_;
