Remove CommandDefinition class
In preparation for traits support, remove CommandDefinition class
and incorporate the missing functionality into CommandDictionary.
BUG: 25841719
Change-Id: Iead48aa0503e9de6061c4c1588b0b930dd82c8d0
Reviewed-on: https://weave-review.googlesource.com/1680
Reviewed-by: Alex Vakulenko <avakulenko@google.com>
diff --git a/src/base_api_handler_unittest.cc b/src/base_api_handler_unittest.cc
index 15a575a..92b83a7 100644
--- a/src/base_api_handler_unittest.cc
+++ b/src/base_api_handler_unittest.cc
@@ -107,8 +107,8 @@
};
TEST_F(BaseApiHandlerTest, Initialization) {
- auto command_defs =
- command_manager_->GetCommandDictionary().GetCommandsAsJson(nullptr);
+ const auto& command_defs =
+ command_manager_->GetCommandDictionary().GetCommandsAsJson();
auto expected = R"({
"base": {
@@ -143,7 +143,7 @@
}
}
})";
- EXPECT_JSON_EQ(expected, *command_defs);
+ EXPECT_JSON_EQ(expected, command_defs);
}
TEST_F(BaseApiHandlerTest, UpdateBaseConfiguration) {
diff --git a/src/commands/cloud_command_proxy_unittest.cc b/src/commands/cloud_command_proxy_unittest.cc
index a65a967..99ddffa 100644
--- a/src/commands/cloud_command_proxy_unittest.cc
+++ b/src/commands/cloud_command_proxy_unittest.cc
@@ -80,6 +80,7 @@
auto json = CreateDictionaryValue(R"({
'calc': {
'add': {
+ 'minimalRole': 'user',
'parameters': {
'value1': 'integer',
'value2': 'integer'
diff --git a/src/commands/command_definition.cc b/src/commands/command_definition.cc
deleted file mode 100644
index dbae630..0000000
--- a/src/commands/command_definition.cc
+++ /dev/null
@@ -1,58 +0,0 @@
-// Copyright 2015 The Weave Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "src/commands/command_definition.h"
-
-#include <vector>
-
-#include <weave/error.h>
-#include <weave/enum_to_string.h>
-
-#include "src/commands/schema_constants.h"
-#include "src/string_utils.h"
-
-namespace weave {
-
-namespace {
-
-const EnumToStringMap<UserRole>::Map kMap[] = {
- {UserRole::kViewer, commands::attributes::kCommand_Role_Viewer},
- {UserRole::kUser, commands::attributes::kCommand_Role_User},
- {UserRole::kOwner, commands::attributes::kCommand_Role_Owner},
- {UserRole::kManager, commands::attributes::kCommand_Role_Manager},
-};
-}
-
-template <>
-LIBWEAVE_EXPORT EnumToStringMap<UserRole>::EnumToStringMap()
- : EnumToStringMap(kMap) {}
-
-CommandDefinition::CommandDefinition(const base::DictionaryValue& definition,
- UserRole minimal_role)
- : minimal_role_{minimal_role} {
- definition_.MergeDictionary(&definition);
-}
-
-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 role: '%s'", value.c_str());
- return definition;
- }
- } else {
- minimal_role = UserRole::kUser;
- }
- 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
deleted file mode 100644
index da02bf5..0000000
--- a/src/commands/command_definition.h
+++ /dev/null
@@ -1,47 +0,0 @@
-// Copyright 2015 The Weave Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef LIBWEAVE_SRC_COMMANDS_COMMAND_DEFINITION_H_
-#define LIBWEAVE_SRC_COMMANDS_COMMAND_DEFINITION_H_
-
-#include <memory>
-#include <string>
-
-#include <base/macros.h>
-#include <base/values.h>
-#include <weave/error.h>
-
-namespace weave {
-
-enum class UserRole {
- kViewer,
- kUser,
- kManager,
- kOwner,
-};
-
-// A simple GCD command definition. This class contains the full object schema
-// describing the command parameter types and constraints.
-class CommandDefinition final {
- public:
- // 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_; }
-
- private:
- CommandDefinition(const base::DictionaryValue& definition,
- UserRole minimal_role);
-
- base::DictionaryValue definition_;
- UserRole minimal_role_;
-
- DISALLOW_COPY_AND_ASSIGN(CommandDefinition);
-};
-
-} // namespace weave
-
-#endif // LIBWEAVE_SRC_COMMANDS_COMMAND_DEFINITION_H_
diff --git a/src/commands/command_definition_unittest.cc b/src/commands/command_definition_unittest.cc
deleted file mode 100644
index 867d48f..0000000
--- a/src/commands/command_definition_unittest.cc
+++ /dev/null
@@ -1,51 +0,0 @@
-// Copyright 2015 The Weave Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "src/commands/command_definition.h"
-
-#include <gtest/gtest.h>
-#include <weave/test/unittest_utils.h>
-
-namespace weave {
-
-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(CommandDefinition, SpecifiedRole) {
- auto params = CreateDictionaryValue(R"({
- 'parameters': {},
- 'progress': {},
- 'results': {},
- 'minimalRole': 'owner'
- })");
- auto def = CommandDefinition::FromJson(*params, nullptr);
- EXPECT_EQ(UserRole::kOwner, def->GetMinimalRole());
-}
-
-TEST(CommandDefinition, IncorrectRole) {
- auto params = CreateDictionaryValue(R"({
- 'parameters': {},
- 'progress': {},
- 'results': {},
- 'minimalRole': 'foo'
- })");
- ErrorPtr error;
- auto def = CommandDefinition::FromJson(*params, &error);
- EXPECT_EQ(nullptr, def.get());
- EXPECT_EQ("invalid_parameter_value", error->GetCode());
-}
-
-} // namespace weave
diff --git a/src/commands/command_dictionary.cc b/src/commands/command_dictionary.cc
index 516961f..f6a409d 100644
--- a/src/commands/command_dictionary.cc
+++ b/src/commands/command_dictionary.cc
@@ -4,42 +4,55 @@
#include "src/commands/command_dictionary.h"
+#include <algorithm>
+
#include <base/values.h>
#include <weave/enum_to_string.h>
-#include "src/commands/command_definition.h"
#include "src/commands/schema_constants.h"
#include "src/string_utils.h"
namespace weave {
+namespace {
+const EnumToStringMap<UserRole>::Map kMap[] = {
+ {UserRole::kViewer, commands::attributes::kCommand_Role_Viewer},
+ {UserRole::kUser, commands::attributes::kCommand_Role_User},
+ {UserRole::kOwner, commands::attributes::kCommand_Role_Owner},
+ {UserRole::kManager, commands::attributes::kCommand_Role_Manager},
+};
+} // anonymous namespace
+
+template <>
+LIBWEAVE_EXPORT EnumToStringMap<UserRole>::EnumToStringMap()
+ : EnumToStringMap(kMap) {}
+
bool CommandDictionary::LoadCommands(const base::DictionaryValue& json,
ErrorPtr* error) {
- CommandMap new_defs;
-
// |json| contains a list of nested objects with the following structure:
// {"<pkg_name>": {"<cmd_name>": {"parameters": {object_schema}}, ...}, ...}
- // Iterate over packages
- base::DictionaryValue::Iterator package_iter(json);
- while (!package_iter.IsAtEnd()) {
- std::string package_name = package_iter.key();
- const base::DictionaryValue* package_value = nullptr;
- if (!package_iter.value().GetAsDictionary(&package_value)) {
+ // Iterate over traits
+ base::DictionaryValue::Iterator trait_iter(json);
+ for (base::DictionaryValue::Iterator trait_iter(json);
+ !trait_iter.IsAtEnd(); trait_iter.Advance()) {
+ std::string trait_name = trait_iter.key();
+ const base::DictionaryValue* trait_def = nullptr;
+ if (!trait_iter.value().GetAsDictionary(&trait_def)) {
Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain,
errors::commands::kTypeMismatch,
- "Expecting an object for package '%s'",
- package_name.c_str());
+ "Expecting an object for trait '%s'",
+ trait_name.c_str());
return false;
}
- // Iterate over command definitions within the current package.
- base::DictionaryValue::Iterator command_iter(*package_value);
- while (!command_iter.IsAtEnd()) {
+ // Iterate over command definitions within the current trait.
+ for (base::DictionaryValue::Iterator command_iter(*trait_def);
+ !command_iter.IsAtEnd(); command_iter.Advance()) {
std::string command_name = command_iter.key();
if (command_name.empty()) {
Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain,
errors::commands::kInvalidCommandName,
- "Unnamed command encountered in package '%s'",
- package_name.c_str());
+ "Unnamed command encountered in trait '%s'",
+ trait_name.c_str());
return false;
}
const base::DictionaryValue* command_def_json = nullptr;
@@ -50,77 +63,89 @@
command_name.c_str());
return false;
}
- // Construct the compound command name as "pkg_name.cmd_name".
- std::string full_command_name = Join(".", package_name, command_name);
- auto command_def = CommandDefinition::FromJson(*command_def_json, error);
- if (!command_def) {
+ // Construct the compound command name as "trait_name.cmd_name".
+ std::string full_command_name = Join(".", trait_name, command_name);
+
+ // Validate the 'minimalRole' value if present. That's the only thing we
+ // care about so far.
+ std::string value;
+ if (!command_def_json->GetString(commands::attributes::kCommand_Role,
+ &value)) {
Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain,
errors::commands::kInvalidMinimalRole,
- "Error parsing command '%s'",
+ "Missing '%s' attribute for command '%s'",
+ commands::attributes::kCommand_Role,
full_command_name.c_str());
return false;
}
-
- new_defs.insert(
- std::make_pair(full_command_name, std::move(command_def)));
- command_iter.Advance();
+ UserRole minimal_role;
+ if (!StringToEnum(value, &minimal_role)) {
+ Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain,
+ errors::commands::kInvalidMinimalRole,
+ "Invalid role '%s' for command '%s'", value.c_str(),
+ full_command_name.c_str());
+ return false;
+ }
+ // Check if we already have this command defined.
+ CHECK(!definitions_.Get(full_command_name, nullptr))
+ << "Definition for command '" << full_command_name
+ << "' overrides an earlier definition";
+ definitions_.Set(full_command_name, command_def_json->DeepCopy());
}
- package_iter.Advance();
}
-
- // Verify that newly loaded command definitions do not override existing
- // definitions in another category. This is unlikely, but we don't want to let
- // one vendor daemon to define the same commands already handled by another
- // daemon on the same device.
- for (const auto& pair : new_defs) {
- auto iter = definitions_.find(pair.first);
- CHECK(iter == definitions_.end()) << "Definition for command '"
- << pair.first
- << "' overrides an earlier definition";
- }
-
- // Insert new definitions into the global map.
- for (auto& pair : new_defs)
- definitions_.insert(std::make_pair(pair.first, std::move(pair.second)));
return true;
}
-std::unique_ptr<base::DictionaryValue> CommandDictionary::GetCommandsAsJson(
- ErrorPtr* error) const {
- std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue);
- for (const auto& pair : definitions_) {
- auto parts = SplitAtFirst(pair.first, ".", true);
- const std::string& package_name = parts.first;
- const std::string& command_name = parts.second;
+const base::DictionaryValue& CommandDictionary::GetCommandsAsJson() const {
+ return definitions_;
+}
- base::DictionaryValue* package = nullptr;
- if (!dict->GetDictionaryWithoutPathExpansion(package_name, &package)) {
- // If this is the first time we encounter this package, create a JSON
- // object for it.
- package = new base::DictionaryValue;
- dict->SetWithoutPathExpansion(package_name, package);
- }
- package->SetWithoutPathExpansion(command_name,
- pair.second->ToJson().DeepCopy());
+size_t CommandDictionary::GetSize() const {
+ size_t size = 0;
+ base::DictionaryValue::Iterator trait_iter(definitions_);
+ while (!trait_iter.IsAtEnd()) {
+ std::string trait_name = trait_iter.key();
+ const base::DictionaryValue* trait_def = nullptr;
+ CHECK(trait_iter.value().GetAsDictionary(&trait_def));
+ size += trait_def->size();
+ trait_iter.Advance();
}
- return dict;
+ return size;
}
-const CommandDefinition* CommandDictionary::FindCommand(
+const base::DictionaryValue* CommandDictionary::FindCommand(
const std::string& command_name) const {
- auto pair = definitions_.find(command_name);
- return (pair != definitions_.end()) ? pair->second.get() : nullptr;
-}
-
-CommandDefinition* CommandDictionary::FindCommand(
- const std::string& command_name) {
- auto pair = definitions_.find(command_name);
- return (pair != definitions_.end()) ? pair->second.get() : nullptr;
+ const base::DictionaryValue* definition = nullptr;
+ // Make sure the |command_name| came in form of trait_name.command_name.
+ // For this, we just verify it has a single period in its name.
+ if (std::count(command_name.begin(), command_name.end(), '.') != 1)
+ return definition;
+ definitions_.GetDictionary(command_name, &definition);
+ return definition;
}
void CommandDictionary::Clear() {
- definitions_.clear();
+ definitions_.Clear();
+}
+
+bool CommandDictionary::GetMinimalRole(const std::string& command_name,
+ UserRole* minimal_role,
+ ErrorPtr* error) const {
+ const base::DictionaryValue* command_def = FindCommand(command_name);
+ if (!command_def) {
+ Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain,
+ errors::commands::kInvalidCommandName,
+ "Command definition for '%s' not found",
+ command_name.c_str());
+ return false;
+ }
+ std::string value;
+ // The JSON definition has been pre-validated already in LoadCommands, so
+ // just using CHECKs here.
+ CHECK(command_def->GetString(commands::attributes::kCommand_Role, &value));
+ CHECK(StringToEnum(value, minimal_role));
+ return true;
}
} // namespace weave
diff --git a/src/commands/command_dictionary.h b/src/commands/command_dictionary.h
index 03e080a..12f7e40 100644
--- a/src/commands/command_dictionary.h
+++ b/src/commands/command_dictionary.h
@@ -12,22 +12,23 @@
#include <vector>
#include <base/macros.h>
+#include <base/values.h>
#include <weave/error.h>
-#include "src/commands/command_definition.h"
-
-namespace base {
-class Value;
-class DictionaryValue;
-} // namespace base
-
namespace weave {
-// 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",
-// where "package_name" is a name of command package such as "base", "printers",
-// and others. So the full command name could be "base.reboot", for example.
+enum class UserRole {
+ kViewer,
+ kUser,
+ kManager,
+ kOwner,
+};
+
+// CommandDictionary is a wrapper around a container of command definition
+// schema. The command name is a compound name in a form of
+// "trait_name.command_name", where "trait_name" is a name of command trait such
+// as "base", "onOff", and others. So the full command name could be
+// "base.reboot", for example.
class CommandDictionary final {
public:
CommandDictionary() = default;
@@ -41,24 +42,24 @@
ErrorPtr* error);
// Converts all the command definitions to a JSON object for CDD/Device
// draft.
- // Returns empty unique_ptr in case of an error and fills in the additional
- // error details in |error|.
- std::unique_ptr<base::DictionaryValue> GetCommandsAsJson(
- ErrorPtr* error) const;
+ const base::DictionaryValue& GetCommandsAsJson() const;
// Returns the number of command definitions in the dictionary.
- size_t GetSize() const { return definitions_.size(); }
+ size_t GetSize() const;
// Checks if the dictionary has no command definitions.
bool IsEmpty() const { return definitions_.empty(); }
// Remove all the command definitions from the dictionary.
void Clear();
// Finds a definition for the given command.
- const CommandDefinition* FindCommand(const std::string& command_name) const;
- CommandDefinition* FindCommand(const std::string& command_name);
+ const base::DictionaryValue* FindCommand(
+ const std::string& command_name) const;
+ // Determines the minimal role for the given command. Returns false if the
+ // command with given name is not found.
+ bool GetMinimalRole(const std::string& command_name,
+ UserRole* minimal_role,
+ ErrorPtr* error) const;
private:
- using CommandMap = std::map<std::string, std::unique_ptr<CommandDefinition>>;
-
- CommandMap definitions_; // List of all available command definitions.
+ base::DictionaryValue definitions_; // List of all available command defs.
DISALLOW_COPY_AND_ASSIGN(CommandDictionary);
};
diff --git a/src/commands/command_dictionary_unittest.cc b/src/commands/command_dictionary_unittest.cc
index adae4ec..7c53935 100644
--- a/src/commands/command_dictionary_unittest.cc
+++ b/src/commands/command_dictionary_unittest.cc
@@ -22,6 +22,7 @@
auto json = CreateDictionaryValue(R"({
'robot': {
'jump': {
+ 'minimalRole': 'manager',
'parameters': {
'height': 'integer',
'_jumpType': ['_withAirFlip', '_withSpin', '_withKick']
@@ -40,9 +41,11 @@
json = CreateDictionaryValue(R"({
'base': {
'reboot': {
+ 'minimalRole': 'owner',
'parameters': {'delay': 'integer'}
},
'shutdown': {
+ 'minimalRole': 'user'
}
}
})");
@@ -75,80 +78,65 @@
EXPECT_FALSE(dict.LoadCommands(*json, &error));
EXPECT_EQ("invalid_command_name", error->GetCode());
error.reset();
+
+ // No 'minimalRole'.
+ json = CreateDictionaryValue(R"({
+ 'base': {
+ 'reboot': {
+ 'parameters': {'delay': 'integer'}
+ }
+ }
+ })");
+ EXPECT_FALSE(dict.LoadCommands(*json, &error));
+ EXPECT_EQ("invalid_minimal_role", error->GetCode());
+ error.reset();
+
+ // Invalid 'minimalRole'.
+ json = CreateDictionaryValue(R"({
+ 'base': {
+ 'reboot': {
+ 'minimalRole': 'foo',
+ 'parameters': {'delay': 'integer'}
+ }
+ }
+ })");
+ EXPECT_FALSE(dict.LoadCommands(*json, &error));
+ EXPECT_EQ("invalid_minimal_role", error->GetCode());
+ error.reset();
}
TEST(CommandDictionaryDeathTest, LoadCommands_Redefine) {
// Redefine commands.
CommandDictionary dict;
ErrorPtr error;
- auto json = CreateDictionaryValue("{'robot':{'jump':{}}}");
+ auto json =
+ CreateDictionaryValue("{'robot':{'jump':{'minimalRole': 'viewer'}}}");
dict.LoadCommands(*json, nullptr);
ASSERT_DEATH(dict.LoadCommands(*json, &error),
".*Definition for command 'robot.jump' overrides an "
"earlier definition");
}
-TEST(CommandDictionary, GetCommandsAsJson) {
- auto json = CreateDictionaryValue(R"({
- 'base': {
- 'reboot': {
- 'parameters': {'delay': {'minimum': 10}},
- 'results': {}
- }
- },
- 'robot': {
- '_jump': {
- 'parameters': {'_height': 'integer'},
- 'minimalRole': 'user'
- }
- }
- })");
- CommandDictionary dict;
- dict.LoadCommands(*json, nullptr);
-
- json = dict.GetCommandsAsJson(nullptr);
- ASSERT_NE(nullptr, json.get());
- auto expected = R"({
- 'base': {
- 'reboot': {
- 'parameters': {'delay': {'minimum': 10}},
- 'results': {}
- }
- },
- 'robot': {
- '_jump': {
- 'parameters': {'_height': 'integer'},
- 'minimalRole': 'user'
- }
- }
- })";
- EXPECT_JSON_EQ(expected, *json);
-}
-
-TEST(CommandDictionary, LoadWithPermissions) {
+TEST(CommandDictionary, GetMinimalRole) {
CommandDictionary base_dict;
auto json = CreateDictionaryValue(R"({
'base': {
'command1': {
- 'parameters': {},
- 'results': {}
- },
- 'command2': {
'minimalRole': 'viewer',
'parameters': {},
'results': {}
},
- 'command3': {
+ 'command2': {
'minimalRole': 'user',
'parameters': {},
'results': {}
},
- 'command4': {
+ 'command3': {
'minimalRole': 'manager',
'parameters': {},
'results': {}
},
- 'command5': {
+ 'command4': {
'minimalRole': 'owner',
'parameters': {},
'results': {}
@@ -156,26 +144,16 @@
}
})");
EXPECT_TRUE(base_dict.LoadCommands(*json, nullptr));
-
- auto cmd = base_dict.FindCommand("base.command1");
- ASSERT_NE(nullptr, cmd);
- EXPECT_EQ(UserRole::kUser, cmd->GetMinimalRole());
-
- cmd = base_dict.FindCommand("base.command2");
- ASSERT_NE(nullptr, cmd);
- EXPECT_EQ(UserRole::kViewer, cmd->GetMinimalRole());
-
- cmd = base_dict.FindCommand("base.command3");
- ASSERT_NE(nullptr, cmd);
- EXPECT_EQ(UserRole::kUser, cmd->GetMinimalRole());
-
- cmd = base_dict.FindCommand("base.command4");
- ASSERT_NE(nullptr, cmd);
- EXPECT_EQ(UserRole::kManager, cmd->GetMinimalRole());
-
- cmd = base_dict.FindCommand("base.command5");
- ASSERT_NE(nullptr, cmd);
- EXPECT_EQ(UserRole::kOwner, cmd->GetMinimalRole());
+ UserRole role;
+ EXPECT_TRUE(base_dict.GetMinimalRole("base.command1", &role, nullptr));
+ EXPECT_EQ(UserRole::kViewer, role);
+ EXPECT_TRUE(base_dict.GetMinimalRole("base.command2", &role, nullptr));
+ EXPECT_EQ(UserRole::kUser, role);
+ EXPECT_TRUE(base_dict.GetMinimalRole("base.command3", &role, nullptr));
+ EXPECT_EQ(UserRole::kManager, role);
+ EXPECT_TRUE(base_dict.GetMinimalRole("base.command4", &role, nullptr));
+ EXPECT_EQ(UserRole::kOwner, role);
+ EXPECT_FALSE(base_dict.GetMinimalRole("base.command5", &role, nullptr));
}
} // namespace weave
diff --git a/src/commands/command_instance.cc b/src/commands/command_instance.cc
index 8328299..c6a9600 100644
--- a/src/commands/command_instance.cc
+++ b/src/commands/command_instance.cc
@@ -9,7 +9,6 @@
#include <weave/error.h>
#include <weave/export.h>
-#include "src/commands/command_definition.h"
#include "src/commands/command_dictionary.h"
#include "src/commands/command_queue.h"
#include "src/commands/schema_constants.h"
@@ -65,12 +64,8 @@
CommandInstance::CommandInstance(const std::string& name,
Command::Origin origin,
- const CommandDefinition* command_definition,
const base::DictionaryValue& parameters)
- : name_{name},
- origin_{origin},
- command_definition_{command_definition} {
- CHECK(command_definition_);
+ : name_{name}, origin_{origin} {
parameters_.MergeDictionary(¶meters);
}
@@ -148,14 +143,12 @@
namespace {
// Helper method to retrieve command parameters from the command definition
-// object passed in as |json| and corresponding command definition schema
-// specified in |command_def|.
+// object passed in as |json|.
// On success, returns |true| and the validated parameters and values through
// |parameters|. Otherwise returns |false| and additional error information in
// |error|.
std::unique_ptr<base::DictionaryValue> GetCommandParameters(
const base::DictionaryValue* json,
- const CommandDefinition* command_def,
ErrorPtr* error) {
// Get the command parameters from 'parameters' property.
std::unique_ptr<base::DictionaryValue> params;
@@ -221,7 +214,7 @@
return instance;
}
- auto parameters = GetCommandParameters(json, command_def, error);
+ auto parameters = GetCommandParameters(json, error);
if (!parameters) {
Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain,
errors::commands::kCommandFailed,
@@ -229,8 +222,7 @@
return instance;
}
- instance.reset(
- new CommandInstance{command_name, origin, command_def, *parameters});
+ instance.reset(new CommandInstance{command_name, origin, *parameters});
if (!command_id->empty())
instance->SetID(*command_id);
diff --git a/src/commands/command_instance.h b/src/commands/command_instance.h
index 32a93a9..82df190 100644
--- a/src/commands/command_instance.h
+++ b/src/commands/command_instance.h
@@ -21,7 +21,6 @@
namespace weave {
-class CommandDefinition;
class CommandDictionary;
class CommandObserver;
class CommandQueue;
@@ -45,7 +44,6 @@
// their values specified in |parameters|.
CommandInstance(const std::string& name,
Command::Origin origin,
- const CommandDefinition* command_definition,
const base::DictionaryValue& parameters);
~CommandInstance() override;
@@ -66,11 +64,6 @@
bool Abort(const Error* command_error, ErrorPtr* error) override;
bool Cancel(ErrorPtr* error) override;
- // Returns command definition.
- const CommandDefinition* GetCommandDefinition() const {
- return command_definition_;
- }
-
// Parses a command instance JSON definition and constructs a CommandInstance
// object, checking the JSON |value| against the command definition schema
// found in command |dictionary|. On error, returns null unique_ptr and
@@ -100,7 +93,6 @@
void DetachFromQueue() {
observers_.Clear();
queue_ = nullptr;
- command_definition_ = nullptr;
}
private:
@@ -118,8 +110,6 @@
std::string name_;
// The origin of the command, either "local" or "cloud".
Command::Origin origin_ = Command::Origin::kLocal;
- // Command definition.
- const CommandDefinition* command_definition_{nullptr};
// Command parameters and their values.
base::DictionaryValue parameters_;
// Current command execution progress.
diff --git a/src/commands/command_instance_unittest.cc b/src/commands/command_instance_unittest.cc
index fb8fe84..7c8aa2d 100644
--- a/src/commands/command_instance_unittest.cc
+++ b/src/commands/command_instance_unittest.cc
@@ -22,12 +22,14 @@
auto json = CreateDictionaryValue(R"({
'base': {
'reboot': {
+ 'minimalRole': 'user',
'parameters': {},
'results': {}
}
},
'robot': {
'jump': {
+ 'minimalRole': 'user',
'parameters': {
'height': {
'type': 'integer',
@@ -43,6 +45,7 @@
'results': {'testResult': 'integer'}
},
'speak': {
+ 'minimalRole': 'user',
'parameters': {
'phrase': {
'type': 'string',
@@ -72,8 +75,7 @@
'phrase': 'iPityDaFool',
'volume': 5
})");
- CommandInstance instance{"robot.speak", Command::Origin::kCloud,
- dict_.FindCommand("robot.speak"), *params};
+ CommandInstance instance{"robot.speak", Command::Origin::kCloud, *params};
EXPECT_TRUE(
instance.Complete(*CreateDictionaryValue("{'foo': 239}"), nullptr));
@@ -85,18 +87,12 @@
instance.GetParameters());
EXPECT_JSON_EQ("{'foo': 239}", instance.GetResults());
- CommandInstance instance2{"base.reboot",
- Command::Origin::kLocal,
- dict_.FindCommand("base.reboot"),
- {}};
+ CommandInstance instance2{"base.reboot", Command::Origin::kLocal, {}};
EXPECT_EQ(Command::Origin::kLocal, instance2.GetOrigin());
}
TEST_F(CommandInstanceTest, SetID) {
- CommandInstance instance{"base.reboot",
- Command::Origin::kLocal,
- dict_.FindCommand("base.reboot"),
- {}};
+ CommandInstance instance{"base.reboot", Command::Origin::kLocal, {}};
instance.SetID("command_id");
EXPECT_EQ("command_id", instance.GetID());
}
diff --git a/src/commands/command_manager.cc b/src/commands/command_manager.cc
index a64c8e5..8d4602b 100644
--- a/src/commands/command_manager.cc
+++ b/src/commands/command_manager.cc
@@ -63,8 +63,11 @@
if (!command_instance)
return false;
- UserRole minimal_role =
- command_instance->GetCommandDefinition()->GetMinimalRole();
+ UserRole minimal_role;
+ if (!GetCommandDictionary().GetMinimalRole(command_instance->GetName(),
+ &minimal_role, error)) {
+ return false;
+ }
if (role < minimal_role) {
Error::AddToPrintf(
error, FROM_HERE, errors::commands::kDomain, "access_denied",
diff --git a/src/commands/command_manager_unittest.cc b/src/commands/command_manager_unittest.cc
index f0dc95d..303cafa 100644
--- a/src/commands/command_manager_unittest.cc
+++ b/src/commands/command_manager_unittest.cc
@@ -24,10 +24,12 @@
const char kTestVendorCommands[] = R"({
"robot": {
"_jump": {
+ "minimalRole": "user",
"parameters": {"height": "integer"},
"results": {}
},
"_speak": {
+ "minimalRole": "user",
"parameters": {"phrase": "string"},
"results": {}
}
@@ -37,6 +39,7 @@
const char kTestTestCommands[] = R"({
"test": {
"_yo": {
+ "minimalRole": "user",
"parameters": {"name": "string"},
"results": {}
}
@@ -63,12 +66,14 @@
auto json_str = R"({
"base": {
"reboot": {
+ "minimalRole": "user",
"parameters": {"delay": "integer"},
"results": {}
}
},
"robot": {
"_jump": {
+ "minimalRole": "user",
"parameters": {"height": "integer"},
"results": {}
}
diff --git a/src/commands/command_queue_unittest.cc b/src/commands/command_queue_unittest.cc
index dc7290a..08e8102 100644
--- a/src/commands/command_queue_unittest.cc
+++ b/src/commands/command_queue_unittest.cc
@@ -12,22 +12,17 @@
#include <base/memory/weak_ptr.h>
#include <gtest/gtest.h>
-#include "src/commands/command_definition.h"
#include "src/string_utils.h"
namespace weave {
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_.get(), {}}};
+ name, Command::Origin::kLocal, {}}};
cmd->SetID(id);
return cmd;
}
@@ -40,9 +35,6 @@
}
CommandQueue queue_;
-
- private:
- std::unique_ptr<CommandDefinition> command_definition_;
};
// Keeps track of commands being added to and removed from the queue_.
diff --git a/src/device_registration_info.cc b/src/device_registration_info.cc
index 0c914b7..30e8862 100644
--- a/src/device_registration_info.cc
+++ b/src/device_registration_info.cc
@@ -21,7 +21,6 @@
#include "src/bind_lambda.h"
#include "src/commands/cloud_command_proxy.h"
-#include "src/commands/command_definition.h"
#include "src/commands/command_manager.h"
#include "src/commands/schema_constants.h"
#include "src/data_encoding.h"
@@ -480,10 +479,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(error);
- if (!commands)
- return nullptr;
+ const base::DictionaryValue& commands =
+ command_manager_->GetCommandDictionary().GetCommandsAsJson();
const base::DictionaryValue& state = state_manager_->GetState();
@@ -505,7 +502,7 @@
channel->SetString("supportedType", "pull");
}
resource->Set("channel", channel.release());
- resource->Set("commandDefs", commands.release());
+ resource->Set("commandDefs", commands.DeepCopy());
resource->Set("state", state.DeepCopy());
return resource;
diff --git a/src/privet/cloud_delegate.cc b/src/privet/cloud_delegate.cc
index c771366..139bfc6 100644
--- a/src/privet/cloud_delegate.cc
+++ b/src/privet/cloud_delegate.cc
@@ -250,10 +250,9 @@
void OnCommandDefChanged() {
command_defs_.Clear();
- auto commands =
- command_manager_->GetCommandDictionary().GetCommandsAsJson(nullptr);
- CHECK(commands);
- command_defs_.MergeDictionary(commands.get());
+ const base::DictionaryValue& commands =
+ command_manager_->GetCommandDictionary().GetCommandsAsJson();
+ command_defs_.MergeDictionary(&commands);
NotifyOnCommandDefsChanged();
}
diff --git a/src/weave_unittest.cc b/src/weave_unittest.cc
index 2e41852..dd10cbe 100644
--- a/src/weave_unittest.cc
+++ b/src/weave_unittest.cc
@@ -47,8 +47,11 @@
const char kCommandDefs[] = R"({
"base": {
- "reboot": {},
+ "reboot": {
+ "minimalRole": "user"
+ },
"_shutdown": {
+ "minimalRole": "user",
"parameters": {},
"results": {}
}
