Add "component" property to command instance
When sending commands, we'll use "component" to route the command to
the target component it was designated for.
As a temporary stop-gap, use "device" as the component name before
we have full implementation of component/trait schema model.
Also removed CommandDictionary from CommandInstance::FromJson since
the validation will be done outside of JSON parsing code in the future
Component Manager class.
BUG: 25841719
Change-Id: I5c649c257fb48ecaaedc1ced84931009f94c2bb3
Reviewed-on: https://weave-review.googlesource.com/1764
Reviewed-by: Vitaly Buka <vitalybuka@google.com>
diff --git a/src/base_api_handler_unittest.cc b/src/base_api_handler_unittest.cc
index 92b83a7..ded6315 100644
--- a/src/base_api_handler_unittest.cc
+++ b/src/base_api_handler_unittest.cc
@@ -72,12 +72,12 @@
void AddCommand(const std::string& command) {
auto command_instance = CommandInstance::FromJson(
test::CreateDictionaryValue(command.c_str()).get(),
- Command::Origin::kLocal, command_manager_->GetCommandDictionary(),
- nullptr, nullptr);
+ Command::Origin::kLocal, nullptr, nullptr);
EXPECT_TRUE(!!command_instance);
std::string id{base::IntToString(++command_id_)};
command_instance->SetID(id);
+ command_instance->SetComponent("device");
command_manager_->AddCommand(std::move(command_instance));
EXPECT_EQ(Command::State::kDone,
command_manager_->FindCommand(id)->GetState());
diff --git a/src/commands/cloud_command_proxy_unittest.cc b/src/commands/cloud_command_proxy_unittest.cc
index 99ddffa..c022b79 100644
--- a/src/commands/cloud_command_proxy_unittest.cc
+++ b/src/commands/cloud_command_proxy_unittest.cc
@@ -76,28 +76,6 @@
EXPECT_CALL(state_change_queue_, GetLastStateChangeId())
.WillRepeatedly(testing::ReturnPointee(¤t_state_update_id_));
- // Set up the command schema.
- auto json = CreateDictionaryValue(R"({
- 'calc': {
- 'add': {
- 'minimalRole': 'user',
- 'parameters': {
- 'value1': 'integer',
- 'value2': 'integer'
- },
- 'progress': {
- 'status' : 'string'
- },
- 'results': {
- 'sum' : 'integer'
- }
- }
- }
- })");
- CHECK(json.get());
- CHECK(command_dictionary_.LoadCommands(*json, nullptr))
- << "Failed to parse test command dictionary";
-
CreateCommandInstance();
}
@@ -112,9 +90,8 @@
})");
CHECK(command_json.get());
- command_instance_ =
- CommandInstance::FromJson(command_json.get(), Command::Origin::kCloud,
- command_dictionary_, nullptr, nullptr);
+ command_instance_ = CommandInstance::FromJson(
+ command_json.get(), Command::Origin::kCloud, nullptr, nullptr);
CHECK(command_instance_.get());
// Backoff - start at 1s and double with each backoff attempt and no jitter.
@@ -139,7 +116,6 @@
testing::StrictMock<MockStateChangeQueueInterface> state_change_queue_;
testing::StrictMock<provider::test::FakeTaskRunner> task_runner_;
std::queue<base::Closure> task_queue_;
- CommandDictionary command_dictionary_;
std::unique_ptr<CommandInstance> command_instance_;
};
diff --git a/src/commands/command_instance.cc b/src/commands/command_instance.cc
index 1f2e4a2..702a819 100644
--- a/src/commands/command_instance.cc
+++ b/src/commands/command_instance.cc
@@ -74,6 +74,10 @@
return name_;
}
+const std::string& CommandInstance::GetComponent() const {
+ return component_;
+}
+
Command::State CommandInstance::GetState() const {
return state_;
}
@@ -169,7 +173,6 @@
std::unique_ptr<CommandInstance> CommandInstance::FromJson(
const base::Value* value,
Command::Origin origin,
- const CommandDictionary& dictionary,
std::string* command_id,
ErrorPtr* error) {
std::unique_ptr<CommandInstance> instance;
@@ -198,14 +201,6 @@
errors::commands::kPropertyMissing, "Command name is missing");
return instance;
}
- // Make sure we know how to handle the command with this name.
- auto command_def = dictionary.FindCommand(command_name);
- if (!command_def) {
- Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain,
- errors::commands::kInvalidCommandName,
- "Unknown command received: %s", command_name.c_str());
- return instance;
- }
auto parameters = GetCommandParameters(json, error);
if (!parameters) {
@@ -220,6 +215,11 @@
if (!command_id->empty())
instance->SetID(*command_id);
+ // Get the component name this command is for.
+ std::string component;
+ if (json->GetString(commands::attributes::kCommand_Component, &component))
+ instance->SetComponent(component);
+
return instance;
}
diff --git a/src/commands/command_instance.h b/src/commands/command_instance.h
index 82df190..15f3ac2 100644
--- a/src/commands/command_instance.h
+++ b/src/commands/command_instance.h
@@ -50,6 +50,7 @@
// Command overrides.
const std::string& GetID() const override;
const std::string& GetName() const override;
+ const std::string& GetComponent() const override;
Command::State GetState() const override;
Command::Origin GetOrigin() const override;
const base::DictionaryValue& GetParameters() const override;
@@ -65,9 +66,8 @@
bool Cancel(ErrorPtr* error) override;
// Parses a command instance JSON definition and constructs a CommandInstance
- // 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|.
+ // object.
+ // On error, returns null unique_ptr and fills in error details in |error|.
// |command_id| is the ID of the command returned, as parsed from the |value|.
// The command ID extracted (if present in the JSON object) even if other
// parsing/validation error occurs and command instance is not constructed.
@@ -75,7 +75,6 @@
static std::unique_ptr<CommandInstance> FromJson(
const base::Value* value,
Command::Origin origin,
- const CommandDictionary& dictionary,
std::string* command_id,
ErrorPtr* error);
@@ -84,6 +83,7 @@
// Sets the command ID (normally done by CommandQueue when the command
// instance is added to it).
void SetID(const std::string& id) { id_ = id; }
+ void SetComponent(const std::string& component) { component_ = component; }
void AddObserver(Observer* observer);
void RemoveObserver(Observer* observer);
@@ -106,8 +106,10 @@
// Unique command ID within a command queue.
std::string id_;
- // Full command name as "<package_name>.<command_name>".
+ // Full command name as "<trait_name>.<command_name>".
std::string name_;
+ // Full path to the component this command is intended for.
+ std::string component_;
// The origin of the command, either "local" or "cloud".
Command::Origin origin_ = Command::Origin::kLocal;
// Command parameters and their values.
diff --git a/src/commands/command_instance_unittest.cc b/src/commands/command_instance_unittest.cc
index 7c8aa2d..6f55ad2 100644
--- a/src/commands/command_instance_unittest.cc
+++ b/src/commands/command_instance_unittest.cc
@@ -14,63 +14,7 @@
using test::CreateDictionaryValue;
using test::CreateValue;
-namespace {
-
-class CommandInstanceTest : public ::testing::Test {
- protected:
- void SetUp() override {
- auto json = CreateDictionaryValue(R"({
- 'base': {
- 'reboot': {
- 'minimalRole': 'user',
- 'parameters': {},
- 'results': {}
- }
- },
- 'robot': {
- 'jump': {
- 'minimalRole': 'user',
- 'parameters': {
- 'height': {
- 'type': 'integer',
- 'minimum': 0,
- 'maximum': 100
- },
- '_jumpType': {
- 'type': 'string',
- 'enum': ['_withAirFlip', '_withSpin', '_withKick']
- }
- },
- 'progress': {'progress': 'integer'},
- 'results': {'testResult': 'integer'}
- },
- 'speak': {
- 'minimalRole': 'user',
- 'parameters': {
- 'phrase': {
- 'type': 'string',
- 'enum': ['beamMeUpScotty', 'iDontDigOnSwine',
- 'iPityDaFool', 'dangerWillRobinson']
- },
- 'volume': {
- 'type': 'integer',
- 'minimum': 0,
- 'maximum': 10
- }
- },
- 'results': {'foo': 'integer'}
- }
- }
- })");
- CHECK(dict_.LoadCommands(*json, nullptr))
- << "Failed to parse test command dictionary";
- }
- CommandDictionary dict_;
-};
-
-} // anonymous namespace
-
-TEST_F(CommandInstanceTest, Test) {
+TEST(CommandInstanceTest, Test) {
auto params = CreateDictionaryValue(R"({
'phrase': 'iPityDaFool',
'volume': 5
@@ -91,15 +35,16 @@
EXPECT_EQ(Command::Origin::kLocal, instance2.GetOrigin());
}
-TEST_F(CommandInstanceTest, SetID) {
+TEST(CommandInstanceTest, SetID) {
CommandInstance instance{"base.reboot", Command::Origin::kLocal, {}};
instance.SetID("command_id");
EXPECT_EQ("command_id", instance.GetID());
}
-TEST_F(CommandInstanceTest, FromJson) {
+TEST(CommandInstanceTest, FromJson) {
auto json = CreateDictionaryValue(R"({
'name': 'robot.jump',
+ 'component': 'comp1.comp2',
'id': 'abcd',
'parameters': {
'height': 53,
@@ -109,64 +54,56 @@
})");
std::string id;
auto instance = CommandInstance::FromJson(json.get(), Command::Origin::kCloud,
- dict_, &id, nullptr);
+ &id, nullptr);
EXPECT_EQ("abcd", id);
EXPECT_EQ("abcd", instance->GetID());
EXPECT_EQ("robot.jump", instance->GetName());
+ EXPECT_EQ("comp1.comp2", instance->GetComponent());
EXPECT_JSON_EQ("{'height': 53, '_jumpType': '_withKick'}",
instance->GetParameters());
}
-TEST_F(CommandInstanceTest, FromJson_ParamsOmitted) {
+TEST(CommandInstanceTest, FromJson_ParamsOmitted) {
auto json = CreateDictionaryValue("{'name': 'base.reboot'}");
auto instance = CommandInstance::FromJson(json.get(), Command::Origin::kCloud,
- dict_, nullptr, nullptr);
+ nullptr, nullptr);
EXPECT_EQ("base.reboot", instance->GetName());
EXPECT_JSON_EQ("{}", instance->GetParameters());
}
-TEST_F(CommandInstanceTest, FromJson_NotObject) {
+TEST(CommandInstanceTest, FromJson_NotObject) {
auto json = CreateValue("'string'");
ErrorPtr error;
auto instance = CommandInstance::FromJson(json.get(), Command::Origin::kCloud,
- dict_, nullptr, &error);
+ nullptr, &error);
EXPECT_EQ(nullptr, instance.get());
EXPECT_EQ("json_object_expected", error->GetCode());
}
-TEST_F(CommandInstanceTest, FromJson_NameMissing) {
+TEST(CommandInstanceTest, FromJson_NameMissing) {
auto json = CreateDictionaryValue("{'param': 'value'}");
ErrorPtr error;
auto instance = CommandInstance::FromJson(json.get(), Command::Origin::kCloud,
- dict_, nullptr, &error);
+ nullptr, &error);
EXPECT_EQ(nullptr, instance.get());
EXPECT_EQ("parameter_missing", error->GetCode());
}
-TEST_F(CommandInstanceTest, FromJson_UnknownCommand) {
- auto json = CreateDictionaryValue("{'name': 'robot.scream'}");
- ErrorPtr error;
- auto instance = CommandInstance::FromJson(json.get(), Command::Origin::kCloud,
- dict_, nullptr, &error);
- EXPECT_EQ(nullptr, instance.get());
- EXPECT_EQ("invalid_command_name", error->GetCode());
-}
-
-TEST_F(CommandInstanceTest, FromJson_ParamsNotObject) {
+TEST(CommandInstanceTest, FromJson_ParamsNotObject) {
auto json = CreateDictionaryValue(R"({
'name': 'robot.speak',
'parameters': 'hello'
})");
ErrorPtr error;
auto instance = CommandInstance::FromJson(json.get(), Command::Origin::kCloud,
- dict_, nullptr, &error);
+ nullptr, &error);
EXPECT_EQ(nullptr, instance.get());
auto inner = error->GetInnerError();
EXPECT_EQ("json_object_expected", inner->GetCode());
EXPECT_EQ("command_failed", error->GetCode());
}
-TEST_F(CommandInstanceTest, ToJson) {
+TEST(CommandInstanceTest, ToJson) {
auto json = CreateDictionaryValue(R"({
'name': 'robot.jump',
'parameters': {
@@ -176,7 +113,7 @@
'results': {}
})");
auto instance = CommandInstance::FromJson(json.get(), Command::Origin::kCloud,
- dict_, nullptr, nullptr);
+ nullptr, nullptr);
EXPECT_TRUE(instance->SetProgress(*CreateDictionaryValue("{'progress': 15}"),
nullptr));
EXPECT_TRUE(instance->SetProgress(*CreateDictionaryValue("{'progress': 15}"),
@@ -198,13 +135,13 @@
*json, *converted);
}
-TEST_F(CommandInstanceTest, ToJsonError) {
+TEST(CommandInstanceTest, ToJsonError) {
auto json = CreateDictionaryValue(R"({
'name': 'base.reboot',
'parameters': {}
})");
auto instance = CommandInstance::FromJson(json.get(), Command::Origin::kCloud,
- dict_, nullptr, nullptr);
+ nullptr, nullptr);
instance->SetID("testId");
ErrorPtr error;
diff --git a/src/commands/command_manager.cc b/src/commands/command_manager.cc
index 8d4602b..9e9852b 100644
--- a/src/commands/command_manager.cc
+++ b/src/commands/command_manager.cc
@@ -57,9 +57,8 @@
UserRole role,
std::string* id,
ErrorPtr* error) {
- auto command_instance =
- CommandInstance::FromJson(&command, Command::Origin::kLocal,
- GetCommandDictionary(), nullptr, error);
+ auto command_instance = CommandInstance::FromJson(
+ &command, Command::Origin::kLocal, nullptr, error);
if (!command_instance)
return false;
@@ -76,6 +75,7 @@
return false;
}
+ command_instance->SetComponent("device");
*id = std::to_string(++next_command_id_);
command_instance->SetID(*id);
AddCommand(std::move(command_instance));
@@ -101,7 +101,7 @@
const Device::CommandHandlerCallback& callback) {
CHECK(command_name.empty() || dictionary_.FindCommand(command_name))
<< "Command undefined: " << command_name;
- command_queue_.AddCommandHandler(command_name, callback);
+ command_queue_.AddCommandHandler("device", command_name, callback);
}
} // namespace weave
diff --git a/src/commands/command_manager.h b/src/commands/command_manager.h
index 04f49ee..644c165 100644
--- a/src/commands/command_manager.h
+++ b/src/commands/command_manager.h
@@ -44,10 +44,6 @@
// Returns the command definitions for the device.
const CommandDictionary& GetCommandDictionary() const;
- // Same as the overload above, but takes a path to a json file to read
- // the base command definitions from.
- bool LoadStandardCommands(const std::string& json, ErrorPtr* error);
-
// Loads device command schema.
bool LoadCommands(const base::DictionaryValue& dict,
ErrorPtr* error);
diff --git a/src/commands/command_queue.cc b/src/commands/command_queue.cc
index 3d2167f..134dc1c 100644
--- a/src/commands/command_queue.cc
+++ b/src/commands/command_queue.cc
@@ -11,6 +11,11 @@
namespace {
const int kRemoveCommandDelayMin = 5;
+
+std::string GetCommandHandlerKey(const std::string& component_path,
+ const std::string& command_name) {
+ return component_path + ":" + command_name;
+}
}
void CommandQueue::AddCommandAddedCallback(const CommandCallback& callback) {
@@ -25,6 +30,7 @@
}
void CommandQueue::AddCommandHandler(
+ const std::string& component_path,
const std::string& command_name,
const Device::CommandHandlerCallback& callback) {
if (!command_name.empty()) {
@@ -33,20 +39,24 @@
for (const auto& command : map_) {
if (command.second->GetState() == Command::State::kQueued &&
- command.second->GetName() == command_name) {
+ command.second->GetName() == command_name &&
+ command.second->GetComponent() == component_path) {
callback.Run(command.second);
}
}
- CHECK(command_callbacks_.insert(std::make_pair(command_name, callback))
- .second)
+ std::string key = GetCommandHandlerKey(component_path, command_name);
+ CHECK(command_callbacks_.insert(std::make_pair(key, callback)).second)
<< command_name << " already has handler";
} else {
+ CHECK(component_path.empty())
+ << "Default handler must not be component-specific";
for (const auto& command : map_) {
+ std::string key = GetCommandHandlerKey(command.second->GetComponent(),
+ command.second->GetName());
if (command.second->GetState() == Command::State::kQueued &&
- command_callbacks_.find(command.second->GetName()) ==
- command_callbacks_.end()) {
+ command_callbacks_.find(key) == command_callbacks_.end()) {
callback.Run(command.second);
}
}
@@ -66,7 +76,9 @@
for (const auto& cb : on_command_added_)
cb.Run(pair.first->second.get());
- auto it_handler = command_callbacks_.find(pair.first->second->GetName());
+ std::string key = GetCommandHandlerKey(pair.first->second->GetComponent(),
+ pair.first->second->GetName());
+ auto it_handler = command_callbacks_.find(key);
if (it_handler != command_callbacks_.end())
it_handler->second.Run(pair.first->second);
diff --git a/src/commands/command_queue.h b/src/commands/command_queue.h
index 74ed86f..0f0a18b 100644
--- a/src/commands/command_queue.h
+++ b/src/commands/command_queue.h
@@ -34,7 +34,8 @@
// Adds notifications callback for a command is removed from the queue.
void AddCommandRemovedCallback(const CommandCallback& callback);
- void AddCommandHandler(const std::string& command_name,
+ void AddCommandHandler(const std::string& component_path,
+ const std::string& command_name,
const Device::CommandHandlerCallback& callback);
// Checks if the command queue is empty.
diff --git a/src/commands/schema_constants.cc b/src/commands/schema_constants.cc
index cd8bf84..34d6db8 100644
--- a/src/commands/schema_constants.cc
+++ b/src/commands/schema_constants.cc
@@ -26,6 +26,7 @@
const char kCommand_Id[] = "id";
const char kCommand_Name[] = "name";
+const char kCommand_Component[] = "component";
const char kCommand_Parameters[] = "parameters";
const char kCommand_Progress[] = "progress";
const char kCommand_Results[] = "results";
diff --git a/src/commands/schema_constants.h b/src/commands/schema_constants.h
index 57766e6..9199480 100644
--- a/src/commands/schema_constants.h
+++ b/src/commands/schema_constants.h
@@ -29,6 +29,7 @@
// Command description JSON schema attributes.
extern const char kCommand_Id[];
extern const char kCommand_Name[];
+extern const char kCommand_Component[];
extern const char kCommand_Parameters[];
extern const char kCommand_Progress[];
extern const char kCommand_Results[];
diff --git a/src/device_registration_info.cc b/src/device_registration_info.cc
index 33b1643..9469d09 100644
--- a/src/device_registration_info.cc
+++ b/src/device_registration_info.cc
@@ -1103,8 +1103,7 @@
std::string command_id;
ErrorPtr error;
auto command_instance = CommandInstance::FromJson(
- &command, Command::Origin::kCloud,
- command_manager_->GetCommandDictionary(), &command_id, &error);
+ &command, Command::Origin::kCloud, &command_id, &error);
if (!command_instance) {
LOG(WARNING) << "Failed to parse a command instance: " << command;
if (!command_id.empty())
