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_;