buffet: Add command ID to CommandInstance class
Rather than passing command ID along with CommandInstance class,
make it to be part of CommandInstance itself. When a command
instance is added to CommandQueue, the command queue will generate
a new ID and set it to the command instance.
Because CommandInstance, when saved in CommandQueue, is now mutable,
remove 'const' in a bunch of places to allow the command instance
to be modifiable.
BUG=chromium:374864
TEST=USE=buffet P2_TEST_FILTER="buffet::*" FEATURES=test emerge-link platform2
Change-Id: Ia30dd4c9bd86b51694d9345dd91f6ed2ae0cb138
Reviewed-on: https://chromium-review.googlesource.com/213266
Reviewed-by: Chris Sosa <sosa@chromium.org>
Tested-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Alex Vakulenko <avakulenko@chromium.org>
diff --git a/buffet/commands/command_dispatch_interface.h b/buffet/commands/command_dispatch_interface.h
index b09c6ec..d1a727d 100644
--- a/buffet/commands/command_dispatch_interface.h
+++ b/buffet/commands/command_dispatch_interface.h
@@ -18,12 +18,10 @@
public:
virtual ~CommandDispachInterface() = default;
// Callback invoked by CommandQueue when a new command is added to the queue.
- virtual void OnCommandAdded(const std::string& command_id,
- const CommandInstance* command_instance) = 0;
+ virtual void OnCommandAdded(CommandInstance* command_instance) = 0;
// Callback invoked by CommandQueue when a new command is removed from
// the queue.
- virtual void OnCommandRemoved(const std::string& command_id,
- const CommandInstance* command_instance) = 0;
+ virtual void OnCommandRemoved(CommandInstance* command_instance) = 0;
};
} // namespace buffet
diff --git a/buffet/commands/command_instance.cc b/buffet/commands/command_instance.cc
index 04b08ab..7c1502c 100644
--- a/buffet/commands/command_instance.cc
+++ b/buffet/commands/command_instance.cc
@@ -72,11 +72,11 @@
} // anonymous namespace
-std::unique_ptr<const CommandInstance> CommandInstance::FromJson(
+std::unique_ptr<CommandInstance> CommandInstance::FromJson(
const base::Value* value,
const CommandDictionary& dictionary,
chromeos::ErrorPtr* error) {
- std::unique_ptr<const CommandInstance> instance;
+ std::unique_ptr<CommandInstance> instance;
// Get the command JSON object from the value.
const base::DictionaryValue* json = nullptr;
if (!value->GetAsDictionary(&json)) {
@@ -114,7 +114,8 @@
return instance;
}
- instance.reset(new CommandInstance(command_name, command_def->GetCategory(),
+ instance.reset(new CommandInstance(command_name,
+ command_def->GetCategory(),
parameters));
return instance;
}
diff --git a/buffet/commands/command_instance.h b/buffet/commands/command_instance.h
index a901380..ebc43ad 100644
--- a/buffet/commands/command_instance.h
+++ b/buffet/commands/command_instance.h
@@ -32,6 +32,8 @@
const std::string& category,
const native_types::Object& parameters);
+ // Returns the full command ID.
+ const std::string& GetID() const { return id_; }
// Returns the full name of the command.
const std::string& GetName() const { return name_; }
// Returns the command category.
@@ -46,12 +48,18 @@
// object, checking the JSON |value| against the command definition schema
// found in command |dictionary|. On error, returns null unique_ptr and
// fills in error details in |error|.
- static std::unique_ptr<const CommandInstance> FromJson(
+ static std::unique_ptr<CommandInstance> FromJson(
const base::Value* value,
const CommandDictionary& dictionary,
chromeos::ErrorPtr* error);
+ // Sets the command ID (normally done by CommandQueue when the command
+ // instance is added to it).
+ void SetID(const std::string& id) { id_ = id; }
+
private:
+ // Unique command ID within a command queue.
+ std::string id_;
// Full command name as "<package_name>.<command_name>".
std::string name_;
// Command category. See comments for CommandDefinitions::LoadCommands for the
diff --git a/buffet/commands/command_instance_unittest.cc b/buffet/commands/command_instance_unittest.cc
index ace0738..844ac92 100644
--- a/buffet/commands/command_instance_unittest.cc
+++ b/buffet/commands/command_instance_unittest.cc
@@ -69,6 +69,7 @@
params.insert(std::make_pair("volume", int_prop.CreateValue(100)));
buffet::CommandInstance instance("robot._beep", "robotd", params);
+ EXPECT_EQ("", instance.GetID());
EXPECT_EQ("robot._beep", instance.GetName());
EXPECT_EQ("robotd", instance.GetCategory());
EXPECT_EQ(params, instance.GetParameters());
@@ -78,6 +79,12 @@
EXPECT_EQ(nullptr, instance.FindParameter("blah").get());
}
+TEST(CommandInstance, SetID) {
+ buffet::CommandInstance instance("robot._beep", "robotd", {});
+ instance.SetID("command_id");
+ EXPECT_EQ("command_id", instance.GetID());
+}
+
TEST_F(CommandInstanceTest, FromJson) {
auto json = CreateDictionaryValue(R"({
'name': 'robot.jump',
diff --git a/buffet/commands/command_queue.cc b/buffet/commands/command_queue.cc
index 671db6d..557a303 100644
--- a/buffet/commands/command_queue.cc
+++ b/buffet/commands/command_queue.cc
@@ -7,31 +7,32 @@
namespace buffet {
-std::string CommandQueue::Add(std::unique_ptr<const CommandInstance> instance) {
+std::string CommandQueue::Add(std::unique_ptr<CommandInstance> instance) {
std::string id = std::to_string(++next_id_);
+ instance->SetID(id);
auto pair = map_.insert(std::make_pair(id, std::move(instance)));
LOG_IF(FATAL, !pair.second) << "Command with ID '" << id
<< "' is already in the queue";
if (dispatch_interface_)
- dispatch_interface_->OnCommandAdded(id, pair.first->second.get());
+ dispatch_interface_->OnCommandAdded(pair.first->second.get());
return id;
}
-std::unique_ptr<const CommandInstance> CommandQueue::Remove(
+std::unique_ptr<CommandInstance> CommandQueue::Remove(
const std::string& id) {
- std::unique_ptr<const CommandInstance> instance;
+ std::unique_ptr<CommandInstance> instance;
auto p = map_.find(id);
if (p != map_.end()) {
instance = std::move(p->second);
map_.erase(p);
if (dispatch_interface_)
- dispatch_interface_->OnCommandRemoved(id, instance.get());
+ dispatch_interface_->OnCommandRemoved(instance.get());
}
return instance;
}
-const CommandInstance* CommandQueue::Find(const std::string& id) const {
+CommandInstance* CommandQueue::Find(const std::string& id) const {
auto p = map_.find(id);
return (p != map_.end()) ? p->second.get() : nullptr;
}
diff --git a/buffet/commands/command_queue.h b/buffet/commands/command_queue.h
index e70fb8d..4e2706b 100644
--- a/buffet/commands/command_queue.h
+++ b/buffet/commands/command_queue.h
@@ -39,21 +39,21 @@
// has no relation to any GCD command identifiers or anything else. Just a
// unique key in this queue class.
// The ID string of the added command is returned by this method.
- std::string Add(std::unique_ptr<const CommandInstance> instance);
+ std::string Add(std::unique_ptr<CommandInstance> instance);
// Removes a command identified by |id| from the queue. Returns a unique
// pointer to the command instance if removed successfully, or an empty
// unique_ptr if the command with the given ID doesn't exist in the queue.
- std::unique_ptr<const CommandInstance> Remove(const std::string& id);
+ std::unique_ptr<CommandInstance> Remove(const std::string& id);
// Finds a command instance in the queue by the instance |id|. Returns
// nullptr if the command with the given |id| is not found. The returned
// pointer should not be persisted for a long period of time.
- const CommandInstance* Find(const std::string& id) const;
+ CommandInstance* Find(const std::string& id) const;
private:
// ID-to-CommandInstance map.
- std::map<std::string, std::unique_ptr<const CommandInstance>> map_;
+ std::map<std::string, std::unique_ptr<CommandInstance>> map_;
// Counter for generating unique command IDs.
int next_id_ = 0;
// Callback interface for command dispatch, if provided.
diff --git a/buffet/commands/command_queue_unittest.cc b/buffet/commands/command_queue_unittest.cc
index a9d5ad9..ce81ea4 100644
--- a/buffet/commands/command_queue_unittest.cc
+++ b/buffet/commands/command_queue_unittest.cc
@@ -14,9 +14,9 @@
namespace {
-std::unique_ptr<const buffet::CommandInstance> CreateDummyCommandInstance(
+std::unique_ptr<buffet::CommandInstance> CreateDummyCommandInstance(
const std::string& name = "base.reboot") {
- return std::unique_ptr<const buffet::CommandInstance>(
+ return std::unique_ptr<buffet::CommandInstance>(
new buffet::CommandInstance(name, "powerd", {}));
}
@@ -25,20 +25,16 @@
// Aborts if duplicate commands are added or non-existent commands are removed.
class FakeDispatchInterface : public buffet::CommandDispachInterface {
public:
- void OnCommandAdded(
- const std::string& command_id,
- const buffet::CommandInstance* command_instance) override {
- CHECK(ids_.insert(command_id).second)
- << "Command ID already exists: " << command_id;
+ void OnCommandAdded(buffet::CommandInstance* command_instance) override {
+ CHECK(ids_.insert(command_instance->GetID()).second)
+ << "Command ID already exists: " << command_instance->GetID();
CHECK(commands_.insert(command_instance).second)
<< "Command instance already exists";
}
- void OnCommandRemoved(
- const std::string& command_id,
- const buffet::CommandInstance* command_instance) override {
- CHECK_EQ(1, ids_.erase(command_id))
- << "Command ID not found: " << command_id;
+ void OnCommandRemoved(buffet::CommandInstance* command_instance) override {
+ CHECK_EQ(1, ids_.erase(command_instance->GetID()))
+ << "Command ID not found: " << command_instance->GetID();
CHECK_EQ(1, commands_.erase(command_instance))
<< "Command instance not found";
}
@@ -52,7 +48,7 @@
private:
std::set<std::string> ids_;
- std::set<const buffet::CommandInstance*> commands_;
+ std::set<buffet::CommandInstance*> commands_;
};
} // anonymous namespace
@@ -116,7 +112,9 @@
auto cmd1 = queue.Find(id1);
EXPECT_NE(nullptr, cmd1);
EXPECT_EQ("base.reboot", cmd1->GetName());
+ EXPECT_EQ(id1, cmd1->GetID());
auto cmd2 = queue.Find(id2);
EXPECT_NE(nullptr, cmd2);
EXPECT_EQ("base.shutdown", cmd2->GetName());
+ EXPECT_EQ(id2, cmd2->GetID());
}
