libweave: Switch DBusCommandDispacher to weave::Command interface
Preparation to be this out of libweave.
BUG=brillo:1245
TEST='FEATURES=test emerge-gizmo buffet'
Change-Id: Ic78881468e2690b22169e54726bbc74f83839251
Reviewed-on: https://chromium-review.googlesource.com/288251
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Tested-by: Vitaly Buka <vitalybuka@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
diff --git a/libweave/include/weave/commands.h b/libweave/include/weave/commands.h
index 2937cbe..820fb44 100644
--- a/libweave/include/weave/commands.h
+++ b/libweave/include/weave/commands.h
@@ -24,6 +24,15 @@
class Commands {
public:
+ using OnCommandCallback = base::Callback<void(Command*)>;
+
+ // Adds notification callback for a new command being added to the queue.
+ virtual void AddOnCommandAddedCallback(const OnCommandCallback& callback) = 0;
+
+ // Adds notification callback for a command being removed from the queue.
+ virtual void AddOnCommandRemovedCallback(
+ const OnCommandCallback& callback) = 0;
+
// Adds a new command to the command queue.
virtual bool AddCommand(const base::DictionaryValue& command,
UserRole role,
diff --git a/libweave/src/base_api_handler.cc b/libweave/src/base_api_handler.cc
index 72f9dd4..feb1ab6 100644
--- a/libweave/src/base_api_handler.cc
+++ b/libweave/src/base_api_handler.cc
@@ -20,7 +20,7 @@
&BaseApiHandler::OnCommandAdded, weak_ptr_factory_.GetWeakPtr()));
}
-void BaseApiHandler::OnCommandAdded(CommandInstance* command) {
+void BaseApiHandler::OnCommandAdded(Command* command) {
if (command->GetStatus() != CommandInstance::kStatusQueued)
return;
@@ -31,7 +31,7 @@
return UpdateDeviceInfo(command);
}
-void BaseApiHandler::UpdateBaseConfiguration(CommandInstance* command) {
+void BaseApiHandler::UpdateBaseConfiguration(Command* command) {
command->SetProgress(base::DictionaryValue{}, nullptr);
const BuffetConfig& config{device_info_->GetConfig()};
@@ -61,7 +61,7 @@
command->Done();
}
-void BaseApiHandler::UpdateDeviceInfo(CommandInstance* command) {
+void BaseApiHandler::UpdateDeviceInfo(Command* command) {
command->SetProgress(base::DictionaryValue{}, nullptr);
const BuffetConfig& config{device_info_->GetConfig()};
diff --git a/libweave/src/base_api_handler.h b/libweave/src/base_api_handler.h
index 8d9add7..7aad3c4 100644
--- a/libweave/src/base_api_handler.h
+++ b/libweave/src/base_api_handler.h
@@ -12,7 +12,7 @@
namespace weave {
-class CommandInstance;
+class Command;
class CommandManager;
class DeviceRegistrationInfo;
class StateManager;
@@ -30,9 +30,9 @@
const std::shared_ptr<CommandManager>& command_manager);
private:
- void OnCommandAdded(CommandInstance* command);
- void UpdateBaseConfiguration(CommandInstance* command);
- void UpdateDeviceInfo(CommandInstance* command);
+ void OnCommandAdded(Command* command);
+ void UpdateBaseConfiguration(Command* command);
+ void UpdateDeviceInfo(Command* command);
base::WeakPtr<DeviceRegistrationInfo> device_info_;
std::shared_ptr<StateManager> state_manager_;
diff --git a/libweave/src/commands/command_manager.cc b/libweave/src/commands/command_manager.cc
index a312051..8f85647 100644
--- a/libweave/src/commands/command_manager.cc
+++ b/libweave/src/commands/command_manager.cc
@@ -23,11 +23,7 @@
CommandManager::CommandManager(
const base::WeakPtr<ExportedObjectManager>& object_manager)
- : command_dispatcher_(object_manager) {
- command_queue_.AddOnCommandAddedCallback(
- base::Bind(&DBusCommandDispacher::OnCommandAdded,
- base::Unretained(&command_dispatcher_)));
-}
+ : command_dispatcher_(object_manager, this) {}
CommandManager::~CommandManager() {}
@@ -168,12 +164,12 @@
}
void CommandManager::AddOnCommandAddedCallback(
- const CommandQueue::Callback& callback) {
+ const OnCommandCallback& callback) {
command_queue_.AddOnCommandAddedCallback(callback);
}
void CommandManager::AddOnCommandRemovedCallback(
- const CommandQueue::Callback& callback) {
+ const OnCommandCallback& callback) {
command_queue_.AddOnCommandRemovedCallback(callback);
}
diff --git a/libweave/src/commands/command_manager.h b/libweave/src/commands/command_manager.h
index c894f71..53942d2 100644
--- a/libweave/src/commands/command_manager.h
+++ b/libweave/src/commands/command_manager.h
@@ -47,6 +47,8 @@
std::string* id,
chromeos::ErrorPtr* error) override;
CommandInstance* FindCommand(const std::string& id) override;
+ void AddOnCommandAddedCallback(const OnCommandCallback& callback) override;
+ void AddOnCommandRemovedCallback(const OnCommandCallback& callback) override;
// Sets callback which is called when command definitions is changed.
void AddOnCommandDefChanged(const base::Closure& callback);
@@ -100,17 +102,11 @@
CommandDefinition::Visibility visibility,
chromeos::ErrorPtr* error);
- // Adds notifications callback for a new command being added to the queue.
- void AddOnCommandAddedCallback(const CommandQueue::Callback& callback);
-
- // Adds notifications callback for a command being removed from the queue.
- void AddOnCommandRemovedCallback(const CommandQueue::Callback& callback);
-
private:
CommandDictionary base_dictionary_; // Base/std command definitions/schemas.
CommandDictionary dictionary_; // Command definitions/schemas.
- DBusCommandDispacher command_dispatcher_;
CommandQueue command_queue_;
+ DBusCommandDispacher command_dispatcher_;
std::vector<base::Callback<void()>> on_command_changed_;
uint32_t next_command_id_{0};
diff --git a/libweave/src/commands/command_queue.cc b/libweave/src/commands/command_queue.cc
index 6b63a29..8031715 100644
--- a/libweave/src/commands/command_queue.cc
+++ b/libweave/src/commands/command_queue.cc
@@ -13,14 +13,16 @@
const int kRemoveCommandDelayMin = 5;
}
-void CommandQueue::AddOnCommandAddedCallback(const Callback& callback) {
+void CommandQueue::AddOnCommandAddedCallback(
+ const Commands::OnCommandCallback& callback) {
on_command_added_.push_back(callback);
// Send all pre-existed commands.
for (const auto& command : map_)
callback.Run(command.second.get());
}
-void CommandQueue::AddOnCommandRemovedCallback(const Callback& callback) {
+void CommandQueue::AddOnCommandRemovedCallback(
+ const Commands::OnCommandCallback& callback) {
on_command_removed_.push_back(callback);
}
diff --git a/libweave/src/commands/command_queue.h b/libweave/src/commands/command_queue.h
index 307b61c..1fbc294 100644
--- a/libweave/src/commands/command_queue.h
+++ b/libweave/src/commands/command_queue.h
@@ -16,19 +16,19 @@
#include <base/macros.h>
#include "libweave/src/commands/command_instance.h"
+#include "weave/commands.h"
namespace weave {
class CommandQueue final {
public:
- using Callback = base::Callback<void(CommandInstance*)>;
CommandQueue() = default;
// Adds notifications callback for a new command is added to the queue.
- void AddOnCommandAddedCallback(const Callback& callback);
+ void AddOnCommandAddedCallback(const Commands::OnCommandCallback& callback);
// Adds notifications callback for a command is removed from the queue.
- void AddOnCommandRemovedCallback(const Callback& callback);
+ void AddOnCommandRemovedCallback(const Commands::OnCommandCallback& callback);
// Checks if the command queue is empty.
bool IsEmpty() const { return map_.empty(); }
@@ -75,7 +75,7 @@
// Queue of commands to be removed.
std::queue<std::pair<base::Time, std::string>> remove_queue_;
- using CallbackList = std::vector<Callback>;
+ using CallbackList = std::vector<Commands::OnCommandCallback>;
CallbackList on_command_added_;
CallbackList on_command_removed_;
diff --git a/libweave/src/commands/command_queue_unittest.cc b/libweave/src/commands/command_queue_unittest.cc
index 36f31a8..dd644ac 100644
--- a/libweave/src/commands/command_queue_unittest.cc
+++ b/libweave/src/commands/command_queue_unittest.cc
@@ -56,18 +56,17 @@
&FakeDispatcher::OnCommandRemoved, weak_ptr_factory_.GetWeakPtr()));
}
- void OnCommandAdded(CommandInstance* command_instance) {
- CHECK(ids_.insert(command_instance->GetID()).second)
- << "Command ID already exists: " << command_instance->GetID();
- CHECK(commands_.insert(command_instance).second)
+ void OnCommandAdded(Command* command) {
+ CHECK(ids_.insert(command->GetID()).second) << "Command ID already exists: "
+ << command->GetID();
+ CHECK(commands_.insert(command).second)
<< "Command instance already exists";
}
- void OnCommandRemoved(CommandInstance* command_instance) {
- CHECK_EQ(1u, ids_.erase(command_instance->GetID()))
- << "Command ID not found: " << command_instance->GetID();
- CHECK_EQ(1u, commands_.erase(command_instance))
- << "Command instance not found";
+ void OnCommandRemoved(Command* command) {
+ CHECK_EQ(1u, ids_.erase(command->GetID())) << "Command ID not found: "
+ << command->GetID();
+ CHECK_EQ(1u, commands_.erase(command)) << "Command instance not found";
}
// Get the comma-separated list of command IDs currently accumulated in the
@@ -79,7 +78,7 @@
private:
std::set<std::string> ids_;
- std::set<CommandInstance*> commands_;
+ std::set<Command*> commands_;
base::WeakPtrFactory<FakeDispatcher> weak_ptr_factory_{this};
};
diff --git a/libweave/src/commands/dbus_command_dispatcher.cc b/libweave/src/commands/dbus_command_dispatcher.cc
index 7c7cfdc..e11dbc3 100644
--- a/libweave/src/commands/dbus_command_dispatcher.cc
+++ b/libweave/src/commands/dbus_command_dispatcher.cc
@@ -7,8 +7,8 @@
#include <chromeos/dbus/exported_object_manager.h>
#include "buffet/dbus_constants.h"
-#include "libweave/src/commands/command_instance.h"
#include "libweave/src/commands/dbus_command_proxy.h"
+#include "weave/command.h"
using chromeos::dbus_utils::AsyncEventSequencer;
using chromeos::dbus_utils::ExportedObjectManager;
@@ -16,18 +16,21 @@
namespace weave {
DBusCommandDispacher::DBusCommandDispacher(
- const base::WeakPtr<ExportedObjectManager>& object_manager)
+ const base::WeakPtr<ExportedObjectManager>& object_manager,
+ Commands* command_manager)
: object_manager_{object_manager} {
+ command_manager->AddOnCommandAddedCallback(base::Bind(
+ &DBusCommandDispacher::OnCommandAdded, weak_ptr_factory_.GetWeakPtr()));
}
-void DBusCommandDispacher::OnCommandAdded(CommandInstance* command_instance) {
+void DBusCommandDispacher::OnCommandAdded(Command* command) {
if (!object_manager_)
return;
std::unique_ptr<DBusCommandProxy> proxy{new DBusCommandProxy(
- object_manager_.get(), object_manager_->GetBus(), command_instance,
+ object_manager_.get(), object_manager_->GetBus(), command,
buffet::kCommandServicePathPrefix + std::to_string(++next_id_))};
proxy->RegisterAsync(AsyncEventSequencer::GetDefaultCompletionAction());
- command_instance->AddObserver(proxy.release());
+ command->AddObserver(proxy.release());
}
} // namespace weave
diff --git a/libweave/src/commands/dbus_command_dispatcher.h b/libweave/src/commands/dbus_command_dispatcher.h
index ad5d880..e3c10ae 100644
--- a/libweave/src/commands/dbus_command_dispatcher.h
+++ b/libweave/src/commands/dbus_command_dispatcher.h
@@ -11,6 +11,8 @@
#include <base/macros.h>
#include <base/memory/weak_ptr.h>
+#include "weave/commands.h"
+
namespace chromeos {
namespace dbus_utils {
class ExportedObjectManager;
@@ -32,18 +34,20 @@
public:
explicit DBusCommandDispacher(
const base::WeakPtr<chromeos::dbus_utils::ExportedObjectManager>&
- object_manager);
-
- void OnCommandAdded(CommandInstance* command_instance);
+ object_manager,
+ Commands* command_manager);
private:
+ void OnCommandAdded(Command* command);
+
base::WeakPtr<chromeos::dbus_utils::ExportedObjectManager> object_manager_;
int next_id_{0};
// Default constructor is used in special circumstances such as for testing.
DBusCommandDispacher() = default;
- friend class CommandManager;
+ base::WeakPtrFactory<DBusCommandDispacher> weak_ptr_factory_{this};
+
DISALLOW_COPY_AND_ASSIGN(DBusCommandDispacher);
};
diff --git a/libweave/src/commands/dbus_command_dispatcher_unittest.cc b/libweave/src/commands/dbus_command_dispatcher_unittest.cc
index b91d24f..612605f 100644
--- a/libweave/src/commands/dbus_command_dispatcher_unittest.cc
+++ b/libweave/src/commands/dbus_command_dispatcher_unittest.cc
@@ -35,8 +35,26 @@
} // anonymous namespace
-class DBusCommandDispacherTest : public testing::Test {
+class DBusCommandDispacherTest : public testing::Test, public Commands {
public:
+ void AddOnCommandAddedCallback(const OnCommandCallback& callback) override {
+ command_queue_.AddOnCommandAddedCallback(callback);
+ }
+
+ void AddOnCommandRemovedCallback(const OnCommandCallback& callback) override {
+ command_queue_.AddOnCommandRemovedCallback(callback);
+ }
+
+ MOCK_METHOD4(AddCommand,
+ bool(const base::DictionaryValue&,
+ UserRole,
+ std::string*,
+ chromeos::ErrorPtr*));
+
+ Command* FindCommand(const std::string& id) override {
+ return command_queue_.Find(id);
+ }
+
void SetUp() override {
command_queue_.SetNowForTest(base::Time::Max());
@@ -61,10 +79,7 @@
om_.reset(new chromeos::dbus_utils::ExportedObjectManager(
bus_.get(), kExportedObjectManagerPath));
om_->RegisterAsync(AsyncEventSequencer::GetDefaultCompletionAction());
- command_dispatcher_.reset(new DBusCommandDispacher(om_->AsWeakPtr()));
- command_queue_.AddOnCommandAddedCallback(
- base::Bind(&DBusCommandDispacher::OnCommandAdded,
- base::Unretained(command_dispatcher_.get())));
+ command_dispatcher_.reset(new DBusCommandDispacher(om_->AsWeakPtr(), this));
// Use a mock exported object for command proxy.
mock_exported_command_proxy_ =
new dbus::MockExportedObject(bus_.get(), kCmdObjPath);
diff --git a/libweave/src/commands/dbus_command_proxy.cc b/libweave/src/commands/dbus_command_proxy.cc
index 782a9b3..86450e2 100644
--- a/libweave/src/commands/dbus_command_proxy.cc
+++ b/libweave/src/commands/dbus_command_proxy.cc
@@ -19,8 +19,7 @@
Command* command,
std::string object_path)
: command_{command},
- dbus_object_{object_manager, bus, dbus::ObjectPath{object_path}} {
-}
+ dbus_object_{object_manager, bus, dbus::ObjectPath{object_path}} {}
void DBusCommandProxy::RegisterAsync(
const AsyncEventSequencer::CompletionAction& completion_callback) {
diff --git a/libweave/src/privet/cloud_delegate.cc b/libweave/src/privet/cloud_delegate.cc
index 1b68037..33d8a25 100644
--- a/libweave/src/privet/cloud_delegate.cc
+++ b/libweave/src/privet/cloud_delegate.cc
@@ -32,8 +32,8 @@
const int kMaxSetupRetries = 5;
const int kFirstRetryTimeoutSec = 1;
-CommandInstance* ReturnNotFound(const std::string& command_id,
- chromeos::ErrorPtr* error) {
+Command* ReturnNotFound(const std::string& command_id,
+ chromeos::ErrorPtr* error) {
chromeos::Error::AddToPrintf(error, FROM_HERE, errors::kDomain,
errors::kNotFound, "Command not found, ID='%s'",
command_id.c_str());
@@ -235,12 +235,12 @@
}
private:
- void OnCommandAdded(CommandInstance* command) {
+ void OnCommandAdded(Command* command) {
// Set to 0 for any new unknown command.
command_owners_.emplace(command->GetID(), 0);
}
- void OnCommandRemoved(CommandInstance* command) {
+ void OnCommandRemoved(Command* command) {
CHECK(command_owners_.erase(command->GetID()));
}
@@ -312,9 +312,9 @@
setup_state_ = SetupState(SetupState::kSuccess);
}
- CommandInstance* GetCommandInternal(const std::string& command_id,
- const UserInfo& user_info,
- chromeos::ErrorPtr* error) const {
+ Command* GetCommandInternal(const std::string& command_id,
+ const UserInfo& user_info,
+ chromeos::ErrorPtr* error) const {
if (user_info.scope() != AuthScope::kOwner) {
auto it = command_owners_.find(command_id);
if (it == command_owners_.end())
