Remove object schema parsing in CommandDefinition
The only thing we now care about in CommandDefinition is the
"minimalRole" field. Everything else is a black-box which we just
forward to the server without any semantic parsing.
Also completely removed command visibility support since it no
longer applies to trait/component model.
BUG: 25841230
Change-Id: Ie8fff57ffada289caa7876c2a53150bb116fd65b
Reviewed-on: https://weave-review.googlesource.com/1617
Reviewed-by: Vitaly Buka <vitalybuka@google.com>
diff --git a/examples/daemon/ledflasher/ledflasher.cc b/examples/daemon/ledflasher/ledflasher.cc
index 38314f5..0019d2c 100644
--- a/examples/daemon/ledflasher/ledflasher.cc
+++ b/examples/daemon/ledflasher/ledflasher.cc
@@ -25,7 +25,7 @@
device_ = device;
device->AddStateDefinitionsFromJson(R"({
- "_ledflasher": {"_leds": {"items": "boolean"}}
+ "_ledflasher": {"_leds": {"type": "array", "items": {"type": "boolean"}}}
})");
device->SetStatePropertiesFromJson(R"({
@@ -37,13 +37,13 @@
"_ledflasher": {
"_set":{
"parameters": {
- "_led": {"minimum": 1, "maximum": 3},
- "_on": "boolean"
+ "_led": {"type": "integer", "minimum": 1, "maximum": 3},
+ "_on": {"type": "boolean"}
}
},
"_toggle":{
"parameters": {
- "_led": {"minimum": 1, "maximum": 3}
+ "_led": {"type": "integer", "minimum": 1, "maximum": 3}
}
}
}
diff --git a/examples/daemon/light/light.cc b/examples/daemon/light/light.cc
index d54de93..a7eb9b3 100644
--- a/examples/daemon/light/light.cc
+++ b/examples/daemon/light/light.cc
@@ -18,31 +18,31 @@
device_ = device;
device->AddStateDefinitionsFromJson(R"({
- "onOff": {"state": ["on", "standby"]},
- "brightness": {"brightness": "integer"},
+ "onOff": {"state": {"type": "string", "enum": ["on", "standby"]}},
+ "brightness": {"brightness": {"type": "integer"}},
"colorXY": {
"colorSetting": {
"properties": {
- "colorX": {"minimum": 0.0, "maximum": 1.0},
- "colorY": {"minimum": 0.0, "maximum": 1.0}
+ "colorX": {"type": "number", "minimum": 0.0, "maximum": 1.0},
+ "colorY": {"type": "number", "minimum": 0.0, "maximum": 1.0}
}
},
"colorCapRed": {
"properties": {
- "colorX": {"minimum": 0.0, "maximum": 1.0},
- "colorY": {"minimum": 0.0, "maximum": 1.0}
+ "colorX": {"type": "number", "minimum": 0.0, "maximum": 1.0},
+ "colorY": {"type": "number", "minimum": 0.0, "maximum": 1.0}
}
},
"colorCapGreen": {
"properties": {
- "colorX": {"minimum": 0.0, "maximum": 1.0},
- "colorY": {"minimum": 0.0, "maximum": 1.0}
+ "colorX": {"type": "number", "minimum": 0.0, "maximum": 1.0},
+ "colorY": {"type": "number", "minimum": 0.0, "maximum": 1.0}
}
},
"colorCapBlue": {
"properties": {
- "colorX": {"minimum": 0.0, "maximum": 1.0},
- "colorY": {"minimum": 0.0, "maximum": 1.0}
+ "colorX": {"type": "number", "minimum": 0.0, "maximum": 1.0},
+ "colorY": {"type": "number", "minimum": 0.0, "maximum": 1.0}
}
}
}
@@ -64,7 +64,7 @@
"onOff": {
"setConfig":{
"parameters": {
- "state": ["on", "standby"]
+ "state": {"type": "string", "enum": ["on", "standby"]}
}
}
},
diff --git a/examples/daemon/lock/lock.cc b/examples/daemon/lock/lock.cc
index 3014fb1..7d941c6 100644
--- a/examples/daemon/lock/lock.cc
+++ b/examples/daemon/lock/lock.cc
@@ -35,8 +35,11 @@
device->AddStateDefinitionsFromJson(R"({
"lock": {
- "lockedState": ["locked", "unlocked", "partiallyLocked"],
- "isLockingSupported": "boolean"}
+ "lockedState": {
+ "type": "string",
+ "enum": ["locked", "unlocked", "partiallyLocked"],
+ }
+ "isLockingSupported": {"type": "boolean"}}
})");
device->SetStatePropertiesFromJson(R"({
@@ -51,7 +54,7 @@
"lock": {
"setConfig":{
"parameters": {
- "lockedState": ["locked", "unlocked"]
+ "lockedState": {"type": "string", "enum":["locked", "unlocked"]}
}
}
}
diff --git a/examples/daemon/sample/sample.cc b/examples/daemon/sample/sample.cc
index 905a977..e065b06 100644
--- a/examples/daemon/sample/sample.cc
+++ b/examples/daemon/sample/sample.cc
@@ -27,9 +27,9 @@
"_hello": {
"minimalRole": "user",
"parameters": {
- "_name": "string"
+ "_name": {"type": "string"}
},
- "results": { "_reply": "string" }
+ "results": { "_reply": {"type": "string"} }
},
"_ping": {
"minimalRole": "user",
@@ -38,16 +38,16 @@
"_countdown": {
"minimalRole": "user",
"parameters": {
- "_seconds": {"minimum": 1, "maximum": 25}
+ "_seconds": {"type": "integer", "minimum": 1, "maximum": 25}
},
- "progress": { "_seconds_left": "integer"},
+ "progress": { "_seconds_left": {"type": "integer"}},
"results": {}
}
}
})");
device->AddStateDefinitionsFromJson(R"({
- "_sample": {"_ping_count":"integer"}
+ "_sample": {"_ping_count": {"type": "integer"}}
})");
device->SetStatePropertiesFromJson(R"({
diff --git a/examples/daemon/speaker/speaker.cc b/examples/daemon/speaker/speaker.cc
index 32591f9..178be14 100644
--- a/examples/daemon/speaker/speaker.cc
+++ b/examples/daemon/speaker/speaker.cc
@@ -18,10 +18,10 @@
device_ = device;
device->AddStateDefinitionsFromJson(R"({
- "onOff": {"state": ["on", "standby"]},
+ "onOff": {"state": {"type": "string", "enum": ["on", "standby"]}},
"volume": {
- "volume": "integer",
- "isMuted": "boolean"
+ "volume": {"type": "integer"},
+ "isMuted": {"type": "boolean"}
}
})");
@@ -38,7 +38,7 @@
"onOff": {
"setConfig":{
"parameters": {
- "state": ["on", "standby"]
+ "state": {"type": "string", "enum": ["on", "standby"]}
}
}
},
@@ -50,7 +50,7 @@
"minimum": 0,
"maximum": 100
},
- "isMuted": "boolean"
+ "isMuted": {"type": "boolean"}
}
}
}
diff --git a/src/base_api_handler.cc b/src/base_api_handler.cc
index c3aa616..3d22a10 100644
--- a/src/base_api_handler.cc
+++ b/src/base_api_handler.cc
@@ -42,20 +42,31 @@
"updateBaseConfiguration": {
"minimalRole": "manager",
"parameters": {
- "localDiscoveryEnabled": "boolean",
- "localAnonymousAccessMaxRole": [ "none", "viewer", "user" ],
- "localPairingEnabled": "boolean"
- },
- "results": {}
+ "localAnonymousAccessMaxRole": {
+ "enum": [ "none", "viewer", "user" ],
+ "type": "string"
+ },
+ "localDiscoveryEnabled": {
+ "type": "boolean"
+ },
+ "localPairingEnabled": {
+ "type": "boolean"
+ }
+ }
},
"updateDeviceInfo": {
"minimalRole": "manager",
"parameters": {
- "description": "string",
- "name": "string",
- "location": "string"
- },
- "results": {}
+ "description": {
+ "type": "string"
+ },
+ "location": {
+ "type": "string"
+ },
+ "name": {
+ "type": "string"
+ }
+ }
}
}
})");
diff --git a/src/base_api_handler_unittest.cc b/src/base_api_handler_unittest.cc
index a025e44..2975bdd 100644
--- a/src/base_api_handler_unittest.cc
+++ b/src/base_api_handler_unittest.cc
@@ -107,39 +107,38 @@
TEST_F(BaseApiHandlerTest, Initialization) {
auto command_defs =
- command_manager_->GetCommandDictionary().GetCommandsAsJson(
- [](const CommandDefinition* def) { return true; }, true, nullptr);
+ command_manager_->GetCommandDictionary().GetCommandsAsJson(nullptr);
auto expected = R"({
"base": {
"updateBaseConfiguration": {
- "minimalRole": "manager",
- "parameters": {
- "localAnonymousAccessMaxRole": {
- "enum": [ "none", "viewer", "user" ],
- "type": "string"
- },
- "localDiscoveryEnabled": {
- "type": "boolean"
- },
- "localPairingEnabled": {
- "type": "boolean"
- }
- }
+ "minimalRole": "manager",
+ "parameters": {
+ "localAnonymousAccessMaxRole": {
+ "enum": [ "none", "viewer", "user" ],
+ "type": "string"
+ },
+ "localDiscoveryEnabled": {
+ "type": "boolean"
+ },
+ "localPairingEnabled": {
+ "type": "boolean"
+ }
+ }
},
"updateDeviceInfo": {
- "minimalRole": "manager",
- "parameters": {
- "description": {
- "type": "string"
- },
- "location": {
- "type": "string"
- },
- "name": {
- "type": "string"
- }
- }
+ "minimalRole": "manager",
+ "parameters": {
+ "description": {
+ "type": "string"
+ },
+ "location": {
+ "type": "string"
+ },
+ "name": {
+ "type": "string"
+ }
+ }
}
}
})";
diff --git a/src/commands/cloud_command_proxy_unittest.cc b/src/commands/cloud_command_proxy_unittest.cc
index 0c04592..fdb22fc 100644
--- a/src/commands/cloud_command_proxy_unittest.cc
+++ b/src/commands/cloud_command_proxy_unittest.cc
@@ -94,7 +94,7 @@
}
})");
CHECK(json.get());
- CHECK(command_dictionary_.LoadCommands(*json, nullptr, nullptr))
+ CHECK(command_dictionary_.LoadCommands(*json, nullptr))
<< "Failed to parse test command dictionary";
CreateCommandInstance();
diff --git a/src/commands/command_definition.cc b/src/commands/command_definition.cc
index d7ebc83..dbae630 100644
--- a/src/commands/command_definition.cc
+++ b/src/commands/command_definition.cc
@@ -28,61 +28,31 @@
LIBWEAVE_EXPORT EnumToStringMap<UserRole>::EnumToStringMap()
: EnumToStringMap(kMap) {}
-bool CommandDefinition::Visibility::FromString(const std::string& str,
- ErrorPtr* error) {
- // This special case is useful for places where we want to make a command
- // to ALL clients, even if new clients are added in the future.
- if (str == commands::attributes::kCommand_Visibility_All) {
- local = true;
- cloud = true;
- return true;
- }
+CommandDefinition::CommandDefinition(const base::DictionaryValue& definition,
+ UserRole minimal_role)
+ : minimal_role_{minimal_role} {
+ definition_.MergeDictionary(&definition);
+}
- // Clear any bits first.
- local = false;
- cloud = false;
- if (str == commands::attributes::kCommand_Visibility_None)
- return true;
-
- for (const std::string& value : Split(str, ",", true, true)) {
- if (value == commands::attributes::kCommand_Visibility_Local) {
- local = true;
- } else if (value == commands::attributes::kCommand_Visibility_Cloud) {
- cloud = true;
- } else {
+std::unique_ptr<CommandDefinition> CommandDefinition::FromJson(
+ const base::DictionaryValue& dict, ErrorPtr* error) {
+ std::unique_ptr<CommandDefinition> definition;
+ // Validate the 'minimalRole' value if present. That's the only thing we
+ // care about so far.
+ std::string value;
+ UserRole minimal_role;
+ if (dict.GetString(commands::attributes::kCommand_Role, &value)) {
+ if (!StringToEnum(value, &minimal_role)) {
Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain,
- errors::commands::kInvalidPropValue,
- "Invalid command visibility value '%s'",
- value.c_str());
- return false;
+ errors::commands::kInvalidPropValue,
+ "Invalid role: '%s'", value.c_str());
+ return definition;
}
+ } else {
+ minimal_role = UserRole::kUser;
}
- return true;
-}
-
-std::string CommandDefinition::Visibility::ToString() const {
- if (local && cloud)
- return commands::attributes::kCommand_Visibility_All;
- if (!local && !cloud)
- return commands::attributes::kCommand_Visibility_None;
- if (local)
- return commands::attributes::kCommand_Visibility_Local;
- return commands::attributes::kCommand_Visibility_Cloud;
-}
-
-CommandDefinition::CommandDefinition(
- std::unique_ptr<const ObjectSchema> parameters,
- std::unique_ptr<const ObjectSchema> progress,
- std::unique_ptr<const ObjectSchema> results)
- : parameters_{std::move(parameters)},
- progress_{std::move(progress)},
- results_{std::move(results)} {
- // Set to be available to all clients by default.
- visibility_ = Visibility::GetAll();
-}
-
-void CommandDefinition::SetVisibility(const Visibility& visibility) {
- visibility_ = visibility;
+ definition.reset(new CommandDefinition{dict, minimal_role});
+ return definition;
}
} // namespace weave
diff --git a/src/commands/command_definition.h b/src/commands/command_definition.h
index 3bcc07f..da02bf5 100644
--- a/src/commands/command_definition.h
+++ b/src/commands/command_definition.h
@@ -9,8 +9,8 @@
#include <string>
#include <base/macros.h>
-
-#include "src/commands/object_schema.h"
+#include <base/values.h>
+#include <weave/error.h>
namespace weave {
@@ -25,55 +25,19 @@
// describing the command parameter types and constraints.
class CommandDefinition final {
public:
- struct Visibility {
- Visibility() = default;
- Visibility(bool is_local, bool is_cloud)
- : local{is_local}, cloud{is_cloud} {}
-
- // Converts a comma-separated string of visibility identifiers into the
- // Visibility bitset (|str| is a string like "local,cloud").
- // Special string value "all" is treated as a list of every possible
- // visibility values and "none" to have all the bits cleared.
- bool FromString(const std::string& str, ErrorPtr* error);
-
- // Converts the visibility bitset to a string.
- std::string ToString() const;
-
- static Visibility GetAll() { return Visibility{true, true}; }
- static Visibility GetLocal() { return Visibility{true, false}; }
- static Visibility GetCloud() { return Visibility{false, true}; }
- static Visibility GetNone() { return Visibility{false, false}; }
-
- bool local{false}; // Command is available to local clients.
- bool cloud{false}; // Command is available to cloud clients.
- };
-
- CommandDefinition(std::unique_ptr<const ObjectSchema> parameters,
- std::unique_ptr<const ObjectSchema> progress,
- std::unique_ptr<const ObjectSchema> results);
-
- // Gets the object schema for command parameters.
- const ObjectSchema* GetParameters() const { return parameters_.get(); }
- // Gets the object schema for command progress.
- const ObjectSchema* GetProgress() const { return progress_.get(); }
- // Gets the object schema for command results.
- const ObjectSchema* GetResults() const { return results_.get(); }
- // Returns the command visibility.
- const Visibility& GetVisibility() const { return visibility_; }
- // Changes the command visibility.
- void SetVisibility(const Visibility& visibility);
+ // Factory method to construct a command definition from a JSON dictionary.
+ static std::unique_ptr<CommandDefinition> FromJson(
+ const base::DictionaryValue& dict, ErrorPtr* error);
+ const base::DictionaryValue& ToJson() const { return definition_; }
// Returns the role required to execute command.
UserRole GetMinimalRole() const { return minimal_role_; }
- // Changes the role required to execute command.
- void SetMinimalRole(UserRole minimal_role) { minimal_role_ = minimal_role; }
private:
- std::unique_ptr<const ObjectSchema> parameters_; // Command parameters def.
- std::unique_ptr<const ObjectSchema> progress_; // Command progress def.
- std::unique_ptr<const ObjectSchema> results_; // Command results def.
- Visibility visibility_; // Available to all by default.
- // Minimal role required to execute command.
- UserRole minimal_role_{UserRole::kUser};
+ CommandDefinition(const base::DictionaryValue& definition,
+ UserRole minimal_role);
+
+ base::DictionaryValue definition_;
+ UserRole minimal_role_;
DISALLOW_COPY_AND_ASSIGN(CommandDefinition);
};
diff --git a/src/commands/command_definition_unittest.cc b/src/commands/command_definition_unittest.cc
index 77b2754..ecd6e54 100644
--- a/src/commands/command_definition_unittest.cc
+++ b/src/commands/command_definition_unittest.cc
@@ -6,87 +6,47 @@
#include <gtest/gtest.h>
+#include "src/commands/unittest_utils.h"
+
namespace weave {
-TEST(CommandVisibility, DefaultConstructor) {
- CommandDefinition::Visibility visibility;
- EXPECT_FALSE(visibility.local);
- EXPECT_FALSE(visibility.cloud);
+using test::CreateDictionaryValue;
+
+TEST(CommandDefinition, DefaultRole) {
+ auto params = CreateDictionaryValue(R"({
+ 'parameters': {
+ 'height': 'integer',
+ 'jumpType': ['_withAirFlip', '_withSpin', '_withKick']
+ },
+ 'progress': {'progress': 'integer'},
+ 'results': {'testResult': 'integer'}
+ })");
+ auto def = CommandDefinition::FromJson(*params, nullptr);
+ EXPECT_EQ(UserRole::kUser, def->GetMinimalRole());
}
-TEST(CommandVisibility, InitialState) {
- auto visibility = CommandDefinition::Visibility::GetAll();
- EXPECT_TRUE(visibility.local);
- EXPECT_TRUE(visibility.cloud);
-
- visibility = CommandDefinition::Visibility::GetLocal();
- EXPECT_TRUE(visibility.local);
- EXPECT_FALSE(visibility.cloud);
-
- visibility = CommandDefinition::Visibility::GetCloud();
- EXPECT_FALSE(visibility.local);
- EXPECT_TRUE(visibility.cloud);
-
- visibility = CommandDefinition::Visibility::GetNone();
- EXPECT_FALSE(visibility.local);
- EXPECT_FALSE(visibility.cloud);
+TEST(CommandDefinition, SpecifiedRole) {
+ auto params = CreateDictionaryValue(R"({
+ 'parameters': {},
+ 'progress': {},
+ 'results': {},
+ 'minimalRole': 'owner'
+ })");
+ auto def = CommandDefinition::FromJson(*params, nullptr);
+ EXPECT_EQ(UserRole::kOwner, def->GetMinimalRole());
}
-TEST(CommandVisibility, FromString) {
- CommandDefinition::Visibility visibility;
-
- ASSERT_TRUE(visibility.FromString("local", nullptr));
- EXPECT_TRUE(visibility.local);
- EXPECT_FALSE(visibility.cloud);
-
- ASSERT_TRUE(visibility.FromString("cloud", nullptr));
- EXPECT_FALSE(visibility.local);
- EXPECT_TRUE(visibility.cloud);
-
- ASSERT_TRUE(visibility.FromString("cloud,local", nullptr));
- EXPECT_TRUE(visibility.local);
- EXPECT_TRUE(visibility.cloud);
-
- ASSERT_TRUE(visibility.FromString("none", nullptr));
- EXPECT_FALSE(visibility.local);
- EXPECT_FALSE(visibility.cloud);
-
- ASSERT_TRUE(visibility.FromString("all", nullptr));
- EXPECT_TRUE(visibility.local);
- EXPECT_TRUE(visibility.cloud);
-
- ASSERT_TRUE(visibility.FromString("", nullptr));
- EXPECT_FALSE(visibility.local);
- EXPECT_FALSE(visibility.cloud);
-
+TEST(CommandDefinition, IncorrectRole) {
+ auto params = CreateDictionaryValue(R"({
+ 'parameters': {},
+ 'progress': {},
+ 'results': {},
+ 'minimalRole': 'foo'
+ })");
ErrorPtr error;
- ASSERT_FALSE(visibility.FromString("cloud,all", &error));
+ auto def = CommandDefinition::FromJson(*params, &error);
+ EXPECT_EQ(nullptr, def.get());
EXPECT_EQ("invalid_parameter_value", error->GetCode());
}
-TEST(CommandVisibility, ToString) {
- EXPECT_EQ("none", CommandDefinition::Visibility::GetNone().ToString());
- EXPECT_EQ("local", CommandDefinition::Visibility::GetLocal().ToString());
- EXPECT_EQ("cloud", CommandDefinition::Visibility::GetCloud().ToString());
- EXPECT_EQ("all", CommandDefinition::Visibility::GetAll().ToString());
-}
-
-TEST(CommandDefinition, Test) {
- std::unique_ptr<const ObjectSchema> params{ObjectSchema::Create()};
- std::unique_ptr<const ObjectSchema> progress{ObjectSchema::Create()};
- std::unique_ptr<const ObjectSchema> results{ObjectSchema::Create()};
- const ObjectSchema* param_ptr = params.get();
- const ObjectSchema* progress_ptr = progress.get();
- const ObjectSchema* results_ptr = results.get();
- CommandDefinition def{std::move(params), std::move(progress),
- std::move(results)};
- EXPECT_EQ(param_ptr, def.GetParameters());
- EXPECT_EQ(progress_ptr, def.GetProgress());
- EXPECT_EQ(results_ptr, def.GetResults());
- EXPECT_EQ("all", def.GetVisibility().ToString());
-
- def.SetVisibility(CommandDefinition::Visibility::GetLocal());
- EXPECT_EQ("local", def.GetVisibility().ToString());
-}
-
} // namespace weave
diff --git a/src/commands/command_dictionary.cc b/src/commands/command_dictionary.cc
index 053e7aa..8fb7f43 100644
--- a/src/commands/command_dictionary.cc
+++ b/src/commands/command_dictionary.cc
@@ -14,7 +14,6 @@
namespace weave {
bool CommandDictionary::LoadCommands(const base::DictionaryValue& json,
- const CommandDictionary* base_commands,
ErrorPtr* error) {
CommandMap new_defs;
@@ -54,89 +53,16 @@
// Construct the compound command name as "pkg_name.cmd_name".
std::string full_command_name = Join(".", package_name, command_name);
- const ObjectSchema* base_parameters_def = nullptr;
- const ObjectSchema* base_progress_def = nullptr;
- const ObjectSchema* base_results_def = nullptr;
- // By default make it available to all clients.
- auto visibility = CommandDefinition::Visibility::GetAll();
- UserRole minimal_role{UserRole::kUser};
- if (base_commands) {
- auto cmd = base_commands->FindCommand(full_command_name);
- if (cmd) {
- base_parameters_def = cmd->GetParameters();
- base_progress_def = cmd->GetProgress();
- base_results_def = cmd->GetResults();
- visibility = cmd->GetVisibility();
- minimal_role = cmd->GetMinimalRole();
- }
-
- // If the base command dictionary was provided but the command was not
- // found in it, this must be a custom (vendor) command. GCD spec states
- // that all custom command names must begin with "_". Let's enforce
- // this rule here.
- if (!cmd) {
- if (command_name.front() != '_') {
- Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain,
- errors::commands::kInvalidCommandName,
- "The name of custom command '%s' in package '%s'"
- " must start with '_'",
- command_name.c_str(), package_name.c_str());
- return false;
- }
- }
+ auto command_def = CommandDefinition::FromJson(*command_def_json, error);
+ if (!command_def) {
+ Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain,
+ errors::commands::kInvalidMinimalRole,
+ "Error parsing command '%s'",
+ full_command_name.c_str());
+ return false;
}
- auto parameters_schema = BuildObjectSchema(
- command_def_json, commands::attributes::kCommand_Parameters,
- base_parameters_def, full_command_name, error);
- if (!parameters_schema)
- return false;
-
- auto progress_schema = BuildObjectSchema(
- command_def_json, commands::attributes::kCommand_Progress,
- base_progress_def, full_command_name, error);
- if (!progress_schema)
- return false;
-
- auto results_schema = BuildObjectSchema(
- command_def_json, commands::attributes::kCommand_Results,
- base_results_def, full_command_name, error);
- if (!results_schema)
- return false;
-
- std::string value;
- if (command_def_json->GetString(commands::attributes::kCommand_Visibility,
- &value)) {
- if (!visibility.FromString(value, error)) {
- Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain,
- errors::commands::kInvalidCommandVisibility,
- "Error parsing command '%s'",
- full_command_name.c_str());
- return false;
- }
- }
-
- if (command_def_json->GetString(commands::attributes::kCommand_Role,
- &value)) {
- if (!StringToEnum(value, &minimal_role)) {
- Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain,
- errors::commands::kInvalidPropValue,
- "Invalid role: '%s'", value.c_str());
- Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain,
- errors::commands::kInvalidMinimalRole,
- "Error parsing command '%s'",
- full_command_name.c_str());
- return false;
- }
- }
-
- std::unique_ptr<CommandDefinition> command_def{new CommandDefinition{
- std::move(parameters_schema), std::move(progress_schema),
- std::move(results_schema)}};
- command_def->SetVisibility(visibility);
- command_def->SetMinimalRole(minimal_role);
new_defs.emplace(full_command_name, std::move(command_def));
-
command_iter.Advance();
}
package_iter.Advance();
@@ -159,49 +85,10 @@
return true;
}
-std::unique_ptr<ObjectSchema> CommandDictionary::BuildObjectSchema(
- const base::DictionaryValue* command_def_json,
- const char* property_name,
- const ObjectSchema* base_def,
- const std::string& command_name,
- ErrorPtr* error) {
- auto object_schema = ObjectSchema::Create();
-
- const base::DictionaryValue* schema_def = nullptr;
- if (!command_def_json->GetDictionaryWithoutPathExpansion(property_name,
- &schema_def)) {
- if (base_def)
- return base_def->Clone();
- return object_schema;
- }
-
- if (!object_schema->FromJson(schema_def, base_def, error)) {
- Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain,
- errors::commands::kInvalidObjectSchema,
- "Invalid definition for command '%s'",
- command_name.c_str());
- return {};
- }
-
- return object_schema;
-}
-
std::unique_ptr<base::DictionaryValue> CommandDictionary::GetCommandsAsJson(
- const std::function<bool(const CommandDefinition*)>& filter,
- bool full_schema,
ErrorPtr* error) const {
std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue);
for (const auto& pair : definitions_) {
- // Check if the command definition has the desired visibility.
- // If not, then skip it.
- if (!filter(pair.second.get()))
- continue;
-
- std::unique_ptr<base::DictionaryValue> parameters =
- pair.second->GetParameters()->ToJson(full_schema, true);
- CHECK(parameters);
- // Progress and results are not part of public commandDefs.
-
auto parts = SplitAtFirst(pair.first, ".", true);
const std::string& package_name = parts.first;
const std::string& command_name = parts.second;
@@ -213,12 +100,8 @@
package = new base::DictionaryValue;
dict->SetWithoutPathExpansion(package_name, package);
}
- base::DictionaryValue* command_def = new base::DictionaryValue;
- command_def->Set(commands::attributes::kCommand_Parameters,
- parameters.release());
- command_def->SetString(commands::attributes::kCommand_Role,
- EnumToString(pair.second->GetMinimalRole()));
- package->SetWithoutPathExpansion(command_name, command_def);
+ package->SetWithoutPathExpansion(command_name,
+ pair.second->ToJson().DeepCopy());
}
return dict;
}
diff --git a/src/commands/command_dictionary.h b/src/commands/command_dictionary.h
index 8d3d45c..03e080a 100644
--- a/src/commands/command_dictionary.h
+++ b/src/commands/command_dictionary.h
@@ -23,8 +23,6 @@
namespace weave {
-class ObjectSchema;
-
// CommandDictionary is a wrapper around a map of command name and the
// corresponding command definition schema. The command name (the key in
// the map) is a compound name in a form of "package_name.command_name",
@@ -37,25 +35,15 @@
// Loads command definitions from a JSON object. This is done at the daemon
// startup and whenever a device daemon decides to update its command list.
// |json| is a JSON dictionary that describes the complete commands. Optional
- // |base_commands| parameter specifies the definition of standard GCD commands
- // for parameter schema validation. Can be set to nullptr if no validation is
- // needed. Returns false on failure and |error| provides additional error
- // information when provided.
+ // Returns false on failure and |error| provides additional error information
+ // when provided.
bool LoadCommands(const base::DictionaryValue& json,
- const CommandDictionary* base_commands,
ErrorPtr* error);
// Converts all the command definitions to a JSON object for CDD/Device
// draft.
- // |filter| is a predicate used to filter out the command definitions to
- // be returned by this method. Only command definitions for which the
- // predicate returns true will be included in the resulting JSON.
- // |full_schema| specifies whether full command definitions must be generated
- // (true) for CDD or only overrides from the base schema (false).
// Returns empty unique_ptr in case of an error and fills in the additional
// error details in |error|.
std::unique_ptr<base::DictionaryValue> GetCommandsAsJson(
- const std::function<bool(const CommandDefinition*)>& filter,
- bool full_schema,
ErrorPtr* error) const;
// Returns the number of command definitions in the dictionary.
size_t GetSize() const { return definitions_.size(); }
@@ -70,13 +58,6 @@
private:
using CommandMap = std::map<std::string, std::unique_ptr<CommandDefinition>>;
- std::unique_ptr<ObjectSchema> BuildObjectSchema(
- const base::DictionaryValue* command_def_json,
- const char* property_name,
- const ObjectSchema* base_def,
- const std::string& command_name,
- ErrorPtr* error);
-
CommandMap definitions_; // List of all available command definitions.
DISALLOW_COPY_AND_ASSIGN(CommandDictionary);
};
diff --git a/src/commands/command_dictionary_unittest.cc b/src/commands/command_dictionary_unittest.cc
index 819718f..5cecd76 100644
--- a/src/commands/command_dictionary_unittest.cc
+++ b/src/commands/command_dictionary_unittest.cc
@@ -35,7 +35,7 @@
}
})");
CommandDictionary dict;
- EXPECT_TRUE(dict.LoadCommands(*json, nullptr, nullptr));
+ EXPECT_TRUE(dict.LoadCommands(*json, nullptr));
EXPECT_EQ(1, dict.GetSize());
EXPECT_NE(nullptr, dict.FindCommand("robot.jump"));
json = CreateDictionaryValue(R"({
@@ -47,7 +47,7 @@
}
}
})");
- EXPECT_TRUE(dict.LoadCommands(*json, nullptr, nullptr));
+ EXPECT_TRUE(dict.LoadCommands(*json, nullptr));
EXPECT_EQ(3, dict.GetSize());
EXPECT_NE(nullptr, dict.FindCommand("robot.jump"));
EXPECT_NE(nullptr, dict.FindCommand("base.reboot"));
@@ -55,177 +55,41 @@
EXPECT_EQ(nullptr, dict.FindCommand("foo.bar"));
}
-TEST(CommandDictionary, LoadWithInheritance) {
- auto json = CreateDictionaryValue(R"({
- 'robot': {
- 'jump': {
- 'minimalRole': 'viewer',
- 'visibility':'local',
- 'parameters': {
- 'height': 'integer'
- },
- 'progress': {
- 'progress': 'integer'
- },
- 'results': {
- 'success': 'boolean'
- }
- }
- }
- })");
- CommandDictionary base_dict;
- EXPECT_TRUE(base_dict.LoadCommands(*json, nullptr, nullptr));
- EXPECT_EQ(1, base_dict.GetSize());
- json = CreateDictionaryValue(R"({'robot': {'jump': {}}})");
-
- CommandDictionary dict;
- EXPECT_TRUE(dict.LoadCommands(*json, &base_dict, nullptr));
- EXPECT_EQ(1, dict.GetSize());
-
- auto cmd = dict.FindCommand("robot.jump");
- EXPECT_NE(nullptr, cmd);
-
- EXPECT_EQ("local", cmd->GetVisibility().ToString());
- EXPECT_EQ(UserRole::kViewer, cmd->GetMinimalRole());
-
- EXPECT_JSON_EQ("{'height': {'type': 'integer'}}",
- *cmd->GetParameters()->ToJson(true, true));
- EXPECT_JSON_EQ("{'progress': {'type': 'integer'}}",
- *cmd->GetProgress()->ToJson(true, false));
- EXPECT_JSON_EQ("{'success': {'type': 'boolean'}}",
- *cmd->GetResults()->ToJson(true, false));
-}
-
TEST(CommandDictionary, LoadCommands_Failures) {
CommandDictionary dict;
ErrorPtr error;
// Command definition is not an object.
auto json = CreateDictionaryValue("{'robot':{'jump':0}}");
- EXPECT_FALSE(dict.LoadCommands(*json, nullptr, &error));
+ EXPECT_FALSE(dict.LoadCommands(*json, &error));
EXPECT_EQ("type_mismatch", error->GetCode());
error.reset();
// Package definition is not an object.
json = CreateDictionaryValue("{'robot':'blah'}");
- EXPECT_FALSE(dict.LoadCommands(*json, nullptr, &error));
+ EXPECT_FALSE(dict.LoadCommands(*json, &error));
EXPECT_EQ("type_mismatch", error->GetCode());
error.reset();
- // Invalid command definition is not an object.
- json = CreateDictionaryValue(
- "{'robot':{'jump':{'parameters':{'flip':0},'results':{}}}}");
- EXPECT_FALSE(dict.LoadCommands(*json, nullptr, &error));
- EXPECT_EQ("invalid_object_schema", error->GetCode());
- EXPECT_NE(nullptr, error->GetInnerError()); // Must have additional info.
- error.reset();
-
// Empty command name.
json = CreateDictionaryValue("{'robot':{'':{'parameters':{},'results':{}}}}");
- EXPECT_FALSE(dict.LoadCommands(*json, nullptr, &error));
+ EXPECT_FALSE(dict.LoadCommands(*json, &error));
EXPECT_EQ("invalid_command_name", error->GetCode());
error.reset();
}
-TEST(CommandDictionaryDeathTest, LoadCommands_RedefineInDifferentCategory) {
- // Redefine commands in different category.
+TEST(CommandDictionaryDeathTest, LoadCommands_Redefine) {
+ // Redefine commands.
CommandDictionary dict;
ErrorPtr error;
auto json = CreateDictionaryValue("{'robot':{'jump':{}}}");
- dict.LoadCommands(*json, nullptr, &error);
- ASSERT_DEATH(dict.LoadCommands(*json, nullptr, &error),
+ dict.LoadCommands(*json, nullptr);
+ ASSERT_DEATH(dict.LoadCommands(*json, &error),
".*Definition for command 'robot.jump' overrides an "
"earlier definition");
}
-TEST(CommandDictionary, LoadCommands_CustomCommandNaming) {
- // Custom command must start with '_'.
- CommandDictionary base_dict;
- CommandDictionary dict;
- ErrorPtr error;
- auto json = CreateDictionaryValue(R"({
- 'base': {
- 'reboot': {
- 'parameters': {'delay': 'integer'},
- 'results': {}
- }
- }
- })");
- base_dict.LoadCommands(*json, nullptr, &error);
- EXPECT_TRUE(dict.LoadCommands(*json, &base_dict, &error));
- auto json2 =
- CreateDictionaryValue("{'base':{'jump':{'parameters':{},'results':{}}}}");
- EXPECT_FALSE(dict.LoadCommands(*json2, &base_dict, &error));
- EXPECT_EQ("invalid_command_name", error->GetCode());
- error.reset();
-
- // If the command starts with "_", then it's Ok.
- json2 = CreateDictionaryValue(
- "{'base':{'_jump':{'parameters':{},'results':{}}}}");
- EXPECT_TRUE(dict.LoadCommands(*json2, &base_dict, nullptr));
-}
-
-TEST(CommandDictionary, LoadCommands_RedefineStdCommand) {
- // Redefine commands parameter type.
- CommandDictionary base_dict;
- CommandDictionary dict;
- ErrorPtr error;
- auto json = CreateDictionaryValue(R"({
- 'base': {
- 'reboot': {
- 'parameters': {'delay': 'integer'},
- 'results': {'version': 'integer'}
- }
- }
- })");
- base_dict.LoadCommands(*json, nullptr, &error);
-
- auto json2 = CreateDictionaryValue(R"({
- 'base': {
- 'reboot': {
- 'parameters': {'delay': 'string'},
- 'results': {'version': 'integer'}
- }
- }
- })");
- EXPECT_FALSE(dict.LoadCommands(*json2, &base_dict, &error));
- EXPECT_EQ("invalid_object_schema", error->GetCode());
- EXPECT_EQ("invalid_parameter_definition", error->GetInnerError()->GetCode());
- EXPECT_EQ("param_type_changed", error->GetFirstError()->GetCode());
- error.reset();
-
- auto json3 = CreateDictionaryValue(R"({
- 'base': {
- 'reboot': {
- 'parameters': {'delay': 'integer'},
- 'results': {'version': 'string'}
- }
- }
- })");
- EXPECT_FALSE(dict.LoadCommands(*json3, &base_dict, &error));
- EXPECT_EQ("invalid_object_schema", error->GetCode());
- // TODO(antonm): remove parameter from error below and use some generic.
- EXPECT_EQ("invalid_parameter_definition", error->GetInnerError()->GetCode());
- EXPECT_EQ("param_type_changed", error->GetFirstError()->GetCode());
- error.reset();
-}
-
TEST(CommandDictionary, GetCommandsAsJson) {
- auto json_base = CreateDictionaryValue(R"({
- 'base': {
- 'reboot': {
- 'parameters': {'delay': {'maximum': 100}},
- 'results': {}
- },
- 'shutdown': {
- 'parameters': {},
- 'results': {}
- }
- }
- })");
- CommandDictionary base_dict;
- base_dict.LoadCommands(*json_base, nullptr, nullptr);
-
auto json = CreateDictionaryValue(R"({
'base': {
'reboot': {
@@ -236,21 +100,20 @@
'robot': {
'_jump': {
'parameters': {'_height': 'integer'},
- 'results': {}
+ 'minimalRole': 'user'
}
}
})");
CommandDictionary dict;
- dict.LoadCommands(*json, &base_dict, nullptr);
+ dict.LoadCommands(*json, nullptr);
- json = dict.GetCommandsAsJson(
- [](const CommandDefinition* def) { return true; }, false, nullptr);
+ json = dict.GetCommandsAsJson(nullptr);
ASSERT_NE(nullptr, json.get());
auto expected = R"({
'base': {
'reboot': {
'parameters': {'delay': {'minimum': 10}},
- 'minimalRole': 'user'
+ 'results': {}
}
},
'robot': {
@@ -261,143 +124,6 @@
}
})";
EXPECT_JSON_EQ(expected, *json);
-
- json = dict.GetCommandsAsJson(
- [](const CommandDefinition* def) { return true; }, true, nullptr);
- ASSERT_NE(nullptr, json.get());
- expected = R"({
- 'base': {
- 'reboot': {
- 'parameters': {
- 'delay': {
- 'maximum': 100,
- 'minimum': 10,
- 'type': 'integer'
- }
- },
- 'minimalRole': 'user'
- }
- },
- 'robot': {
- '_jump': {
- 'parameters': {
- '_height': {
- 'type': 'integer'
- }
- },
- 'minimalRole': 'user'
- }
- }
- })";
- EXPECT_JSON_EQ(expected, *json);
-}
-
-TEST(CommandDictionary, GetCommandsAsJsonWithVisibility) {
- auto json = CreateDictionaryValue(R"({
- 'test': {
- 'command1': {
- 'parameters': {},
- 'results': {},
- 'visibility': 'none'
- },
- 'command2': {
- 'parameters': {},
- 'results': {},
- 'visibility': 'local'
- },
- 'command3': {
- 'parameters': {},
- 'results': {},
- 'visibility': 'cloud'
- },
- 'command4': {
- 'parameters': {},
- 'results': {},
- 'visibility': 'all'
- },
- 'command5': {
- 'parameters': {},
- 'results': {},
- 'visibility': 'none'
- },
- 'command6': {
- 'parameters': {},
- 'results': {},
- 'visibility': 'local'
- },
- 'command7': {
- 'parameters': {},
- 'results': {},
- 'visibility': 'cloud'
- },
- 'command8': {
- 'parameters': {},
- 'results': {},
- 'visibility': 'all'
- }
- }
- })");
- CommandDictionary dict;
- ASSERT_TRUE(dict.LoadCommands(*json, nullptr, nullptr));
-
- json = dict.GetCommandsAsJson(
- [](const CommandDefinition* def) { return true; }, false, nullptr);
- ASSERT_NE(nullptr, json.get());
- auto expected = R"({
- 'test': {
- 'command1': {'parameters': {}, 'minimalRole': 'user'},
- 'command2': {'parameters': {}, 'minimalRole': 'user'},
- 'command3': {'parameters': {}, 'minimalRole': 'user'},
- 'command4': {'parameters': {}, 'minimalRole': 'user'},
- 'command5': {'parameters': {}, 'minimalRole': 'user'},
- 'command6': {'parameters': {}, 'minimalRole': 'user'},
- 'command7': {'parameters': {}, 'minimalRole': 'user'},
- 'command8': {'parameters': {}, 'minimalRole': 'user'}
- }
- })";
- EXPECT_JSON_EQ(expected, *json);
-
- json = dict.GetCommandsAsJson(
- [](const CommandDefinition* def) { return def->GetVisibility().local; },
- false, nullptr);
- ASSERT_NE(nullptr, json.get());
- expected = R"({
- 'test': {
- 'command2': {'parameters': {}, 'minimalRole': 'user'},
- 'command4': {'parameters': {}, 'minimalRole': 'user'},
- 'command6': {'parameters': {}, 'minimalRole': 'user'},
- 'command8': {'parameters': {}, 'minimalRole': 'user'}
- }
- })";
- EXPECT_JSON_EQ(expected, *json);
-
- json = dict.GetCommandsAsJson(
- [](const CommandDefinition* def) { return def->GetVisibility().cloud; },
- false, nullptr);
- ASSERT_NE(nullptr, json.get());
- expected = R"({
- 'test': {
- 'command3': {'parameters': {}, 'minimalRole': 'user'},
- 'command4': {'parameters': {}, 'minimalRole': 'user'},
- 'command7': {'parameters': {}, 'minimalRole': 'user'},
- 'command8': {'parameters': {}, 'minimalRole': 'user'}
- }
- })";
- EXPECT_JSON_EQ(expected, *json);
-
- json = dict.GetCommandsAsJson(
- [](const CommandDefinition* def) {
- return def->GetVisibility().local && def->GetVisibility().cloud;
- },
- false, nullptr);
- ASSERT_NE(nullptr, json.get());
- expected = R"({
- 'test': {
- 'command4': {'parameters': {}, 'minimalRole': 'user'},
- 'command8': {'parameters': {}, 'minimalRole': 'user'}
- }
- })";
- EXPECT_JSON_EQ(expected, *json);
}
TEST(CommandDictionary, LoadWithPermissions) {
@@ -406,161 +132,51 @@
'base': {
'command1': {
'parameters': {},
- 'results': {},
- 'visibility':'none'
+ 'results': {}
},
'command2': {
'minimalRole': 'viewer',
'parameters': {},
- 'results': {},
- 'visibility':'local'
+ 'results': {}
},
'command3': {
'minimalRole': 'user',
'parameters': {},
- 'results': {},
- 'visibility':'cloud'
+ 'results': {}
},
'command4': {
'minimalRole': 'manager',
'parameters': {},
- 'results': {},
- 'visibility':'all'
+ 'results': {}
},
'command5': {
'minimalRole': 'owner',
'parameters': {},
- 'results': {},
- 'visibility':'local,cloud'
+ 'results': {}
}
}
})");
- EXPECT_TRUE(base_dict.LoadCommands(*json, nullptr, nullptr));
+ EXPECT_TRUE(base_dict.LoadCommands(*json, nullptr));
auto cmd = base_dict.FindCommand("base.command1");
ASSERT_NE(nullptr, cmd);
- EXPECT_EQ("none", cmd->GetVisibility().ToString());
EXPECT_EQ(UserRole::kUser, cmd->GetMinimalRole());
cmd = base_dict.FindCommand("base.command2");
ASSERT_NE(nullptr, cmd);
- EXPECT_EQ("local", cmd->GetVisibility().ToString());
EXPECT_EQ(UserRole::kViewer, cmd->GetMinimalRole());
cmd = base_dict.FindCommand("base.command3");
ASSERT_NE(nullptr, cmd);
- EXPECT_EQ("cloud", cmd->GetVisibility().ToString());
EXPECT_EQ(UserRole::kUser, cmd->GetMinimalRole());
cmd = base_dict.FindCommand("base.command4");
ASSERT_NE(nullptr, cmd);
- EXPECT_EQ("all", cmd->GetVisibility().ToString());
EXPECT_EQ(UserRole::kManager, cmd->GetMinimalRole());
cmd = base_dict.FindCommand("base.command5");
ASSERT_NE(nullptr, cmd);
- EXPECT_EQ("all", cmd->GetVisibility().ToString());
EXPECT_EQ(UserRole::kOwner, cmd->GetMinimalRole());
-
- CommandDictionary dict;
- json = CreateDictionaryValue(R"({
- 'base': {
- 'command1': {
- 'parameters': {},
- 'results': {}
- },
- 'command2': {
- 'parameters': {},
- 'results': {}
- },
- 'command3': {
- 'parameters': {},
- 'results': {}
- },
- 'command4': {
- 'parameters': {},
- 'results': {}
- },
- 'command5': {
- 'parameters': {},
- 'results': {}
- },
- '_command6': {
- 'parameters': {},
- 'results': {}
- }
- }
- })");
- EXPECT_TRUE(dict.LoadCommands(*json, &base_dict, nullptr));
-
- cmd = dict.FindCommand("base.command1");
- ASSERT_NE(nullptr, cmd);
- EXPECT_EQ("none", cmd->GetVisibility().ToString());
- EXPECT_EQ(UserRole::kUser, cmd->GetMinimalRole());
-
- cmd = dict.FindCommand("base.command2");
- ASSERT_NE(nullptr, cmd);
- EXPECT_EQ("local", cmd->GetVisibility().ToString());
- EXPECT_EQ(UserRole::kViewer, cmd->GetMinimalRole());
-
- cmd = dict.FindCommand("base.command3");
- ASSERT_NE(nullptr, cmd);
- EXPECT_EQ("cloud", cmd->GetVisibility().ToString());
- EXPECT_EQ(UserRole::kUser, cmd->GetMinimalRole());
-
- cmd = dict.FindCommand("base.command4");
- ASSERT_NE(nullptr, cmd);
- EXPECT_EQ("all", cmd->GetVisibility().ToString());
- EXPECT_EQ(UserRole::kManager, cmd->GetMinimalRole());
-
- cmd = dict.FindCommand("base.command5");
- ASSERT_NE(nullptr, cmd);
- EXPECT_EQ("all", cmd->GetVisibility().ToString());
- EXPECT_EQ(UserRole::kOwner, cmd->GetMinimalRole());
-
- cmd = dict.FindCommand("base._command6");
- ASSERT_NE(nullptr, cmd);
- EXPECT_EQ("all", cmd->GetVisibility().ToString());
- EXPECT_EQ(UserRole::kUser, cmd->GetMinimalRole());
-}
-
-TEST(CommandDictionary, LoadWithPermissions_InvalidVisibility) {
- CommandDictionary dict;
- ErrorPtr error;
-
- auto json = CreateDictionaryValue(R"({
- 'base': {
- 'jump': {
- 'parameters': {},
- 'results': {},
- 'visibility':'foo'
- }
- }
- })");
- EXPECT_FALSE(dict.LoadCommands(*json, nullptr, &error));
- EXPECT_EQ("invalid_command_visibility", error->GetCode());
- EXPECT_EQ("invalid_parameter_value", error->GetInnerError()->GetCode());
- error.reset();
-}
-
-TEST(CommandDictionary, LoadWithPermissions_InvalidRole) {
- CommandDictionary dict;
- ErrorPtr error;
-
- auto json = CreateDictionaryValue(R"({
- 'base': {
- 'jump': {
- 'parameters': {},
- 'results': {},
- 'visibility':'local,cloud',
- 'minimalRole':'foo'
- }
- }
- })");
- EXPECT_FALSE(dict.LoadCommands(*json, nullptr, &error));
- EXPECT_EQ("invalid_minimal_role", error->GetCode());
- EXPECT_EQ("invalid_parameter_value", error->GetInnerError()->GetCode());
- error.reset();
}
} // namespace weave
diff --git a/src/commands/command_instance_unittest.cc b/src/commands/command_instance_unittest.cc
index a03e92b..cacb86c 100644
--- a/src/commands/command_instance_unittest.cc
+++ b/src/commands/command_instance_unittest.cc
@@ -59,7 +59,7 @@
}
}
})");
- CHECK(dict_.LoadCommands(*json, nullptr, nullptr))
+ CHECK(dict_.LoadCommands(*json, nullptr))
<< "Failed to parse test command dictionary";
}
CommandDictionary dict_;
diff --git a/src/commands/command_manager.cc b/src/commands/command_manager.cc
index 75d8295..a64c8e5 100644
--- a/src/commands/command_manager.cc
+++ b/src/commands/command_manager.cc
@@ -28,7 +28,7 @@
bool CommandManager::LoadCommands(const base::DictionaryValue& dict,
ErrorPtr* error) {
- bool result = dictionary_.LoadCommands(dict, nullptr, error);
+ bool result = dictionary_.LoadCommands(dict, error);
for (const auto& cb : on_command_changed_)
cb.Run();
return result;
@@ -83,37 +83,6 @@
return command_queue_.Find(id);
}
-bool CommandManager::SetCommandVisibility(
- const std::vector<std::string>& command_names,
- CommandDefinition::Visibility visibility,
- ErrorPtr* error) {
- if (command_names.empty())
- return true;
-
- std::vector<CommandDefinition*> definitions;
- definitions.reserve(command_names.size());
-
- // Find/validate command definitions first.
- for (const std::string& name : command_names) {
- CommandDefinition* def = dictionary_.FindCommand(name);
- if (!def) {
- Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain,
- errors::commands::kInvalidCommandName,
- "Command '%s' is unknown", name.c_str());
- return false;
- }
- definitions.push_back(def);
- }
-
- // Now that we know that all the command names were valid,
- // update the respective commands' visibility.
- for (CommandDefinition* def : definitions)
- def->SetVisibility(visibility);
- for (const auto& cb : on_command_changed_)
- cb.Run();
- return true;
-}
-
void CommandManager::AddCommandAddedCallback(
const CommandQueue::CommandCallback& callback) {
command_queue_.AddCommandAddedCallback(callback);
diff --git a/src/commands/command_manager.h b/src/commands/command_manager.h
index 7cc8907..04f49ee 100644
--- a/src/commands/command_manager.h
+++ b/src/commands/command_manager.h
@@ -60,11 +60,6 @@
// Adds a new command to the command queue.
void AddCommand(std::unique_ptr<CommandInstance> command_instance);
- // Changes the visibility of commands.
- bool SetCommandVisibility(const std::vector<std::string>& command_names,
- CommandDefinition::Visibility visibility,
- ErrorPtr* error);
-
bool AddCommand(const base::DictionaryValue& command,
UserRole role,
std::string* id,
diff --git a/src/commands/command_manager_unittest.cc b/src/commands/command_manager_unittest.cc
index 0c890a9..eca5175 100644
--- a/src/commands/command_manager_unittest.cc
+++ b/src/commands/command_manager_unittest.cc
@@ -91,59 +91,4 @@
EXPECT_NE(nullptr, manager.GetCommandDictionary().FindCommand("test._yo"));
}
-TEST(CommandManager, UpdateCommandVisibility) {
- CommandManager manager;
- int update_count = 0;
- auto on_command_change = [&update_count]() { update_count++; };
- manager.AddCommandDefChanged(base::Bind(on_command_change));
-
- auto json = CreateDictionaryValue(R"({
- 'foo': {
- '_baz': {
- 'parameters': {},
- 'results': {}
- },
- '_bar': {
- 'parameters': {},
- 'results': {}
- }
- },
- 'bar': {
- '_quux': {
- 'parameters': {},
- 'results': {},
- 'visibility': 'none'
- }
- }
- })");
- ASSERT_TRUE(manager.LoadCommands(*json, nullptr));
- EXPECT_EQ(2, update_count);
- const CommandDictionary& dict = manager.GetCommandDictionary();
- EXPECT_TRUE(manager.SetCommandVisibility(
- {"foo._baz"}, CommandDefinition::Visibility::GetLocal(), nullptr));
- EXPECT_EQ(3, update_count);
- EXPECT_EQ("local", dict.FindCommand("foo._baz")->GetVisibility().ToString());
- EXPECT_EQ("all", dict.FindCommand("foo._bar")->GetVisibility().ToString());
- EXPECT_EQ("none", dict.FindCommand("bar._quux")->GetVisibility().ToString());
-
- ErrorPtr error;
- ASSERT_FALSE(manager.SetCommandVisibility(
- {"foo._baz", "foo._bar", "test.cmd"},
- CommandDefinition::Visibility::GetLocal(), &error));
- EXPECT_EQ(errors::commands::kInvalidCommandName, error->GetCode());
- // The visibility state of commands shouldn't have changed.
- EXPECT_EQ(3, update_count);
- EXPECT_EQ("local", dict.FindCommand("foo._baz")->GetVisibility().ToString());
- EXPECT_EQ("all", dict.FindCommand("foo._bar")->GetVisibility().ToString());
- EXPECT_EQ("none", dict.FindCommand("bar._quux")->GetVisibility().ToString());
-
- EXPECT_TRUE(manager.SetCommandVisibility(
- {"foo._baz", "bar._quux"}, CommandDefinition::Visibility::GetCloud(),
- nullptr));
- EXPECT_EQ(4, update_count);
- EXPECT_EQ("cloud", dict.FindCommand("foo._baz")->GetVisibility().ToString());
- EXPECT_EQ("all", dict.FindCommand("foo._bar")->GetVisibility().ToString());
- EXPECT_EQ("cloud", dict.FindCommand("bar._quux")->GetVisibility().ToString());
-}
-
} // namespace weave
diff --git a/src/commands/command_queue_unittest.cc b/src/commands/command_queue_unittest.cc
index 5060071..a9e953e 100644
--- a/src/commands/command_queue_unittest.cc
+++ b/src/commands/command_queue_unittest.cc
@@ -20,11 +20,15 @@
class CommandQueueTest : public testing::Test {
public:
+ CommandQueueTest() {
+ command_definition_ = CommandDefinition::FromJson({}, nullptr);
+ }
+
std::unique_ptr<CommandInstance> CreateDummyCommandInstance(
const std::string& name,
const std::string& id) {
std::unique_ptr<CommandInstance> cmd{new CommandInstance{
- name, Command::Origin::kLocal, &command_definition_, {}}};
+ name, Command::Origin::kLocal, command_definition_.get(), {}}};
cmd->SetID(id);
return cmd;
}
@@ -39,8 +43,7 @@
CommandQueue queue_;
private:
- CommandDefinition command_definition_{
- ObjectSchema::Create(), ObjectSchema::Create(), ObjectSchema::Create()};
+ std::unique_ptr<CommandDefinition> command_definition_;
};
// Keeps track of commands being added to and removed from the queue_.
diff --git a/src/commands/schema_constants.cc b/src/commands/schema_constants.cc
index c99536b..7f8431f 100644
--- a/src/commands/schema_constants.cc
+++ b/src/commands/schema_constants.cc
@@ -23,7 +23,6 @@
const char kDuplicateCommandDef[] = "duplicate_command_definition";
const char kInvalidCommandName[] = "invalid_command_name";
const char kCommandFailed[] = "command_failed";
-const char kInvalidCommandVisibility[] = "invalid_command_visibility";
const char kInvalidMinimalRole[] = "invalid_minimal_role";
const char kCommandDestroyed[] = "command_destroyed";
const char kInvalidState[] = "invalid_state";
@@ -66,12 +65,6 @@
const char kCommand_Role_User[] = "user";
const char kCommand_Role_Viewer[] = "viewer";
-const char kCommand_Visibility[] = "visibility";
-const char kCommand_Visibility_None[] = "none";
-const char kCommand_Visibility_Local[] = "local";
-const char kCommand_Visibility_Cloud[] = "cloud";
-const char kCommand_Visibility_All[] = "all";
-
} // namespace attributes
} // namespace commands
diff --git a/src/commands/schema_constants.h b/src/commands/schema_constants.h
index 742245f..ea59f17 100644
--- a/src/commands/schema_constants.h
+++ b/src/commands/schema_constants.h
@@ -26,7 +26,6 @@
extern const char kDuplicateCommandDef[];
extern const char kInvalidCommandName[];
extern const char kCommandFailed[];
-extern const char kInvalidCommandVisibility[];
extern const char kInvalidMinimalRole[];
extern const char kCommandDestroyed[];
extern const char kInvalidState[];
@@ -69,12 +68,6 @@
extern const char kCommand_Role_User[];
extern const char kCommand_Role_Viewer[];
-extern const char kCommand_Visibility[];
-extern const char kCommand_Visibility_None[];
-extern const char kCommand_Visibility_Local[];
-extern const char kCommand_Visibility_Cloud[];
-extern const char kCommand_Visibility_All[];
-
} // namespace attributes
} // namespace commands
diff --git a/src/device_registration_info.cc b/src/device_registration_info.cc
index 751e530..d60b6cb 100644
--- a/src/device_registration_info.cc
+++ b/src/device_registration_info.cc
@@ -480,9 +480,8 @@
std::unique_ptr<base::DictionaryValue>
DeviceRegistrationInfo::BuildDeviceResource(ErrorPtr* error) {
// Limit only to commands that are visible to the cloud.
- auto commands = command_manager_->GetCommandDictionary().GetCommandsAsJson(
- [](const CommandDefinition* def) { return def->GetVisibility().cloud; },
- true, error);
+ auto commands =
+ command_manager_->GetCommandDictionary().GetCommandsAsJson(error);
if (!commands)
return nullptr;
diff --git a/src/device_registration_info_unittest.cc b/src/device_registration_info_unittest.cc
index df4a438..9b53872 100644
--- a/src/device_registration_info_unittest.cc
+++ b/src/device_registration_info_unittest.cc
@@ -355,16 +355,14 @@
auto json_cmds = CreateDictionaryValue(R"({
'base': {
'reboot': {
- 'parameters': {'delay': {'minimum': 10}},
- 'minimalRole': 'user',
- 'results': {}
+ 'parameters': {'delay': {'minimum': 10, 'type': 'integer'}},
+ 'minimalRole': 'user'
}
},
'robot': {
'_jump': {
- 'parameters': {'_height': 'integer'},
- 'minimalRole': 'user',
- 'results': {}
+ 'parameters': {'_height': {'type': 'integer'}},
+ 'minimalRole': 'user'
}
}
})");
diff --git a/src/privet/cloud_delegate.cc b/src/privet/cloud_delegate.cc
index d612668..32ebf48 100644
--- a/src/privet/cloud_delegate.cc
+++ b/src/privet/cloud_delegate.cc
@@ -251,9 +251,8 @@
void OnCommandDefChanged() {
command_defs_.Clear();
- auto commands = command_manager_->GetCommandDictionary().GetCommandsAsJson(
- [](const CommandDefinition* def) { return def->GetVisibility().local; },
- true, nullptr);
+ auto commands =
+ command_manager_->GetCommandDictionary().GetCommandsAsJson(nullptr);
CHECK(commands);
command_defs_.MergeDictionary(commands.get());
NotifyOnCommandDefsChanged();
diff --git a/src/weave_unittest.cc b/src/weave_unittest.cc
index c2f1e8f..2e41852 100644
--- a/src/weave_unittest.cc
+++ b/src/weave_unittest.cc
@@ -72,7 +72,7 @@
"base": {
"reboot": {
"minimalRole": "user",
- "parameters": {"delay": "integer"},
+ "parameters": {"delay": {"type": "integer"}},
"results": {}
},
"shutdown": {
