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": {