buffet: load standard GCD command definitions in CommandManager
Changed CommandManager to load the base command definitions
and use it as a base schema for loading device-specific commands
and make sure device vendors are not redefining standard commands
in breaking manner.
BUG=chromium:374861
TEST=USE=buffet P2_TEST_FILTER="buffet::*" FEATURES=test emerge-link platform2
Change-Id: I5f2d5500bc90ef918a8a6df1242bdd1fe3b78615
Reviewed-on: https://chromium-review.googlesource.com/209175
Tested-by: Alex Vakulenko <avakulenko@chromium.org>
Reviewed-by: Christopher Wiley <wiley@chromium.org>
Commit-Queue: Alex Vakulenko <avakulenko@chromium.org>
diff --git a/buffet/commands/command_dictionary.cc b/buffet/commands/command_dictionary.cc
index 38c8bce..dcba8c9 100644
--- a/buffet/commands/command_dictionary.cc
+++ b/buffet/commands/command_dictionary.cc
@@ -24,6 +24,7 @@
bool CommandDictionary::LoadCommands(const base::DictionaryValue& json,
const std::string& category,
+ const CommandDictionary* base_commands,
ErrorPtr* error) {
std::map<std::string, std::shared_ptr<const CommandDefinition>> new_defs;
@@ -32,29 +33,37 @@
// Iterate over packages
base::DictionaryValue::Iterator package_iter(json);
while (!package_iter.IsAtEnd()) {
- std::string package = package_iter.key();
+ std::string package_name = package_iter.key();
const base::DictionaryValue* package_value = nullptr;
if (!package_iter.value().GetAsDictionary(&package_value)) {
Error::AddToPrintf(error, errors::commands::kDomain,
errors::commands::kTypeMismatch,
"Expecting an object for package '%s'",
- package.c_str());
+ package_name.c_str());
return false;
}
// Iterate over command definitions within the current package.
base::DictionaryValue::Iterator command_iter(*package_value);
while (!command_iter.IsAtEnd()) {
- std::string command = command_iter.key();
+ std::string command_name = command_iter.key();
+ if (command_name.empty()) {
+ Error::AddToPrintf(error, errors::commands::kDomain,
+ errors::commands::kInvalidCommandName,
+ "Unnamed command encountered in package '%s'",
+ package_name.c_str());
+ return false;
+ }
const base::DictionaryValue* command_value = nullptr;
if (!command_iter.value().GetAsDictionary(&command_value)) {
Error::AddToPrintf(error, errors::commands::kDomain,
errors::commands::kTypeMismatch,
"Expecting an object for command '%s'",
- command.c_str());
+ command_name.c_str());
return false;
}
// Construct the compound command name as "pkg_name.cmd_name".
- std::string command_name = string_utils::Join('.', package, command);
+ std::string full_command_name = string_utils::Join('.', package_name,
+ command_name);
// Get the "parameters" definition of the command and read it into
// an object schema.
const base::DictionaryValue* command_schema_def = nullptr;
@@ -63,21 +72,45 @@
Error::AddToPrintf(error, errors::commands::kDomain,
errors::commands::kPropertyMissing,
"Command definition '%s' is missing property '%s'",
- command_name.c_str(),
+ full_command_name.c_str(),
commands::attributes::kCommand_Parameters);
return false;
}
+
+ const ObjectSchema* base_def = nullptr;
+ if (base_commands) {
+ const CommandDefinition* cmd =
+ base_commands->FindCommand(full_command_name);
+ if (cmd)
+ base_def = cmd->GetParameters().get();
+
+ // 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 (!base_def) {
+ if (command_name.front() != '_') {
+ Error::AddToPrintf(error, 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_schema = std::make_shared<ObjectSchema>();
- if (!command_schema->FromJson(command_schema_def, nullptr, error)) {
+ if (!command_schema->FromJson(command_schema_def, base_def, error)) {
Error::AddToPrintf(error, errors::commands::kDomain,
errors::commands::kInvalidObjectSchema,
"Invalid definition for command '%s'",
- command_name.c_str());
+ full_command_name.c_str());
return false;
}
auto command_def = std::make_shared<CommandDefinition>(category,
command_schema);
- new_defs.insert(std::make_pair(command_name, command_def));
+ new_defs.insert(std::make_pair(full_command_name, command_def));
command_iter.Advance();
}
diff --git a/buffet/commands/command_dictionary.h b/buffet/commands/command_dictionary.h
index 11295c9..78f2cee 100644
--- a/buffet/commands/command_dictionary.h
+++ b/buffet/commands/command_dictionary.h
@@ -46,10 +46,14 @@
// When LoadCommands is called, all previous definitions of commands from the
// same category are removed, effectively replacing all the commands in the
// given category.
+ // 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.
bool LoadCommands(const base::DictionaryValue& json,
- const std::string& category, ErrorPtr* error);
+ const std::string& category,
+ const CommandDictionary* base_commands, ErrorPtr* error);
// Returns the number of command definitions in the dictionary.
size_t GetSize() const { return definitions_.size(); }
// Checks if the dictionary has no command definitions.
diff --git a/buffet/commands/command_dictionary_unittest.cc b/buffet/commands/command_dictionary_unittest.cc
index f345019..0aeab50 100644
--- a/buffet/commands/command_dictionary_unittest.cc
+++ b/buffet/commands/command_dictionary_unittest.cc
@@ -28,7 +28,7 @@
}
})");
buffet::CommandDictionary dict;
- EXPECT_TRUE(dict.LoadCommands(*json, "robotd", nullptr));
+ EXPECT_TRUE(dict.LoadCommands(*json, "robotd", nullptr, nullptr));
EXPECT_EQ(1, dict.GetSize());
EXPECT_NE(nullptr, dict.FindCommand("robot.jump"));
json = CreateDictionaryValue(R"({
@@ -41,7 +41,7 @@
}
}
})");
- EXPECT_TRUE(dict.LoadCommands(*json, "powerd", nullptr));
+ EXPECT_TRUE(dict.LoadCommands(*json, "powerd", nullptr, nullptr));
EXPECT_EQ(3, dict.GetSize());
EXPECT_NE(nullptr, dict.FindCommand("robot.jump"));
EXPECT_NE(nullptr, dict.FindCommand("base.reboot"));
@@ -57,7 +57,7 @@
// Command definition missing 'parameters' property.
auto json = CreateDictionaryValue("{'robot':{'jump':{}}}");
- EXPECT_FALSE(dict.LoadCommands(*json, "robotd", &error));
+ EXPECT_FALSE(dict.LoadCommands(*json, "robotd", nullptr, &error));
EXPECT_EQ("parameter_missing", error->GetCode());
EXPECT_EQ("Command definition 'robot.jump' is missing property 'parameters'",
error->GetMessage());
@@ -65,33 +65,103 @@
// Command definition is not an object.
json = CreateDictionaryValue("{'robot':{'jump':0}}");
- EXPECT_FALSE(dict.LoadCommands(*json, "robotd", &error));
+ EXPECT_FALSE(dict.LoadCommands(*json, "robotd", nullptr, &error));
EXPECT_EQ("type_mismatch", error->GetCode());
EXPECT_EQ("Expecting an object for command 'jump'", error->GetMessage());
error.reset();
// Package definition is not an object.
json = CreateDictionaryValue("{'robot':'blah'}");
- EXPECT_FALSE(dict.LoadCommands(*json, "robotd", &error));
+ EXPECT_FALSE(dict.LoadCommands(*json, "robotd", nullptr, &error));
EXPECT_EQ("type_mismatch", error->GetCode());
EXPECT_EQ("Expecting an object for package 'robot'", error->GetMessage());
error.reset();
// Invalid command definition is not an object.
json = CreateDictionaryValue("{'robot':{'jump':{'parameters':{'flip':0}}}}");
- EXPECT_FALSE(dict.LoadCommands(*json, "robotd", &error));
+ EXPECT_FALSE(dict.LoadCommands(*json, "robotd", nullptr, &error));
EXPECT_EQ("invalid_object_schema", error->GetCode());
EXPECT_EQ("Invalid definition for command 'robot.jump'", error->GetMessage());
EXPECT_NE(nullptr, error->GetInnerError()); // Must have additional info.
error.reset();
+ // Empty command name.
+ json = CreateDictionaryValue("{'robot':{'':{'parameters':{'flip':0}}}}");
+ EXPECT_FALSE(dict.LoadCommands(*json, "robotd", nullptr, &error));
+ EXPECT_EQ("invalid_command_name", error->GetCode());
+ EXPECT_EQ("Unnamed command encountered in package 'robot'",
+ error->GetMessage());
+ error.reset();
+}
+
+TEST(CommandDictionary, LoadCommands_RedefineInDifferentCategory) {
// Redefine commands in different category.
- json = CreateDictionaryValue("{'robot':{'jump':{'parameters':{}}}}");
- dict.Clear();
- dict.LoadCommands(*json, "category1", &error);
- EXPECT_FALSE(dict.LoadCommands(*json, "category2", &error));
+ buffet::CommandDictionary dict;
+ buffet::ErrorPtr error;
+ auto json = CreateDictionaryValue("{'robot':{'jump':{'parameters':{}}}}");
+ dict.LoadCommands(*json, "category1", nullptr, &error);
+ EXPECT_FALSE(dict.LoadCommands(*json, "category2", nullptr, &error));
EXPECT_EQ("duplicate_command_definition", error->GetCode());
EXPECT_EQ("Definition for command 'robot.jump' overrides an earlier "
"definition in category 'category1'", error->GetMessage());
error.reset();
}
+
+TEST(CommandDictionary, LoadCommands_CustomCommandNaming) {
+ // Custom command must start with '_'.
+ buffet::CommandDictionary base_dict;
+ buffet::CommandDictionary dict;
+ buffet::ErrorPtr error;
+ auto json = CreateDictionaryValue(R"({
+ 'base': {
+ 'reboot': {
+ 'parameters': {'delay': 'integer'}
+ }
+ }
+ })");
+ base_dict.LoadCommands(*json, "", nullptr, &error);
+ EXPECT_TRUE(dict.LoadCommands(*json, "robotd", &base_dict, &error));
+ auto json2 = CreateDictionaryValue("{'base':{'jump':{'parameters':{}}}}");
+ EXPECT_FALSE(dict.LoadCommands(*json2, "robotd", &base_dict, &error));
+ EXPECT_EQ("invalid_command_name", error->GetCode());
+ EXPECT_EQ("The name of custom command 'jump' in package 'base' must start "
+ "with '_'", error->GetMessage());
+ error.reset();
+
+ // If the command starts with "_", then it's Ok.
+ json2 = CreateDictionaryValue("{'base':{'_jump':{'parameters':{}}}}");
+ EXPECT_TRUE(dict.LoadCommands(*json2, "robotd", &base_dict, nullptr));
+}
+
+TEST(CommandDictionary, LoadCommands_RedefineStdCommand) {
+ // Redefine commands parameter type.
+ buffet::CommandDictionary base_dict;
+ buffet::CommandDictionary dict;
+ buffet::ErrorPtr error;
+ auto json = CreateDictionaryValue(R"({
+ 'base': {
+ 'reboot': {
+ 'parameters': {'delay': 'integer'}
+ }
+ }
+ })");
+ base_dict.LoadCommands(*json, "", nullptr, &error);
+ auto json2 = CreateDictionaryValue(R"({
+ 'base': {
+ 'reboot': {
+ 'parameters': {'delay': 'string'}
+ }
+ }
+ })");
+ EXPECT_FALSE(dict.LoadCommands(*json2, "robotd", &base_dict, &error));
+ EXPECT_EQ("invalid_object_schema", error->GetCode());
+ EXPECT_EQ("Invalid definition for command 'base.reboot'",
+ error->GetMessage());
+ EXPECT_EQ("invalid_parameter_definition", error->GetInnerError()->GetCode());
+ EXPECT_EQ("Error in definition of property 'delay'",
+ error->GetInnerError()->GetMessage());
+ EXPECT_EQ("param_type_changed", error->GetFirstError()->GetCode());
+ EXPECT_EQ("Redefining a property of type integer as string",
+ error->GetFirstError()->GetMessage());
+ error.reset();
+}
diff --git a/buffet/commands/command_manager.cc b/buffet/commands/command_manager.cc
index fc04b2d..d7cf34c 100644
--- a/buffet/commands/command_manager.cc
+++ b/buffet/commands/command_manager.cc
@@ -4,10 +4,78 @@
#include "buffet/commands/command_manager.h"
+#include <base/file_util.h>
+#include <base/values.h>
+#include <base/json/json_reader.h>
+
+#include "buffet/commands/schema_constants.h"
+#include "buffet/error_codes.h"
+
namespace buffet {
const CommandDictionary& CommandManager::GetCommandDictionary() const {
return dictionary_;
}
+bool CommandManager::LoadBaseCommands(const base::DictionaryValue& json,
+ ErrorPtr* error) {
+ return base_dictionary_.LoadCommands(json, "", nullptr, error);
+}
+
+bool CommandManager::LoadBaseCommands(const base::FilePath& json_file_path,
+ ErrorPtr* error) {
+ std::unique_ptr<const base::DictionaryValue> json = LoadJsonDict(
+ json_file_path, error);
+ if (!json)
+ return false;
+ return LoadBaseCommands(*json, error);
+}
+
+bool CommandManager::LoadCommands(const base::DictionaryValue& json,
+ const std::string& category,
+ ErrorPtr* error) {
+ return dictionary_.LoadCommands(json, category, &base_dictionary_, error);
+}
+
+bool CommandManager::LoadCommands(const base::FilePath& json_file_path,
+ ErrorPtr* error) {
+ std::unique_ptr<const base::DictionaryValue> json = LoadJsonDict(
+ json_file_path, error);
+ if (!json)
+ return false;
+ std::string category = json_file_path.BaseName().RemoveExtension().value();
+ return LoadCommands(*json, category, error);
+}
+
+std::unique_ptr<const base::DictionaryValue> CommandManager::LoadJsonDict(
+ const base::FilePath& json_file_path, ErrorPtr* error) {
+ std::string json_string;
+ if (!base::ReadFileToString(json_file_path, &json_string)) {
+ Error::AddToPrintf(error, errors::file_system::kDomain,
+ errors::file_system::kFileReadError,
+ "Failed to read file '%s'",
+ json_file_path.value().c_str());
+ return std::unique_ptr<const base::DictionaryValue>();
+ }
+ std::string error_message;
+ base::Value* value = base::JSONReader::ReadAndReturnError(
+ json_string, base::JSON_PARSE_RFC, nullptr, &error_message);
+ if (!value) {
+ Error::AddToPrintf(error, errors::json::kDomain, errors::json::kParseError,
+ "Error parsing content of JSON file '%s': %s",
+ json_file_path.value().c_str(), error_message.c_str());
+ return std::unique_ptr<const base::DictionaryValue>();
+ }
+ const base::DictionaryValue* dict_value = nullptr;
+ if (!value->GetAsDictionary(&dict_value)) {
+ delete value;
+ Error::AddToPrintf(error, errors::json::kDomain,
+ errors::json::kObjectExpected,
+ "Content of file '%s' is not a JSON object",
+ json_file_path.value().c_str());
+ return std::unique_ptr<const base::DictionaryValue>();
+ }
+ return std::unique_ptr<const base::DictionaryValue>(dict_value);
+}
+
} // namespace buffet
diff --git a/buffet/commands/command_manager.h b/buffet/commands/command_manager.h
index 1700712..64037b8 100644
--- a/buffet/commands/command_manager.h
+++ b/buffet/commands/command_manager.h
@@ -5,7 +5,10 @@
#ifndef BUFFET_COMMANDS_COMMAND_MANAGER_H_
#define BUFFET_COMMANDS_COMMAND_MANAGER_H_
+#include <string>
+
#include <base/basictypes.h>
+#include <base/files/file_path.h>
#include "buffet/commands/command_dictionary.h"
@@ -21,7 +24,42 @@
// Get the command definitions for the device.
const CommandDictionary& GetCommandDictionary() const;
+ // Loads base/standard GCD command definitions.
+ // |json| is the full JSON schema of standard GCD commands. These commands
+ // are not necessarily supported by a particular device but rather
+ // all the standard commands defined by GCD standard for all known/supported
+ // device kinds.
+ // On success, returns true. Otherwise, |error| contains additional
+ // error information.
+ bool LoadBaseCommands(const base::DictionaryValue& json,
+ ErrorPtr* error);
+
+ // Same as the overload above, but takes a path to a json file to read
+ // the base command definitions from.
+ bool LoadBaseCommands(const base::FilePath& json_file_path,
+ ErrorPtr* error);
+
+ // Loads device command schema for particular category.
+ // See CommandDictionary::LoadCommands for detailed description of the
+ // parameters.
+ bool LoadCommands(const base::DictionaryValue& json,
+ const std::string& category, ErrorPtr* error);
+
+ // Same as the overload above, but takes a path to a json file to read
+ // the base command definitions from. Also, the command category is
+ // derived from file name (without extension). So, if the path points to
+ // "power_manager.json", the command category used will be "power_manager".
+ bool LoadCommands(const base::FilePath& json_file_path,
+ ErrorPtr* error);
+
private:
+ // Helper function to load a JSON file that is expected to be
+ // an object/dictionary. In case of error, returns empty unique ptr and fills
+ // in error details in |error|.
+ std::unique_ptr<const base::DictionaryValue> LoadJsonDict(
+ const base::FilePath& json_file_path, ErrorPtr* error);
+
+ CommandDictionary base_dictionary_; // Base/std command definitions/schemas.
CommandDictionary dictionary_; // Command definitions/schemas.
DISALLOW_COPY_AND_ASSIGN(CommandManager);
};
diff --git a/buffet/commands/command_manager_unittest.cc b/buffet/commands/command_manager_unittest.cc
index 1a0bfec..65241fe 100644
--- a/buffet/commands/command_manager_unittest.cc
+++ b/buffet/commands/command_manager_unittest.cc
@@ -2,11 +2,107 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include <base/file_util.h>
+#include <base/json/json_writer.h>
#include <gtest/gtest.h>
#include "buffet/commands/command_manager.h"
+#include "buffet/commands/unittest_utils.h"
+
+using buffet::unittests::CreateDictionaryValue;
+
+static base::FilePath SaveJsonToTempFile(const base::DictionaryValue& dict) {
+ std::string json;
+ base::JSONWriter::Write(&dict, &json);
+ base::FilePath temp_file;
+ base::CreateTemporaryFile(&temp_file);
+ base::WriteFile(temp_file, json.data(), static_cast<int>(json.size()));
+ return temp_file;
+}
TEST(CommandManager, Empty) {
buffet::CommandManager manager;
EXPECT_TRUE(manager.GetCommandDictionary().IsEmpty());
}
+
+TEST(CommandManager, LoadBaseCommandsJSON) {
+ buffet::CommandManager manager;
+ auto json = CreateDictionaryValue(R"({
+ 'base': {
+ 'reboot': {
+ 'parameters': {'delay': 'integer'}
+ },
+ 'shutdown': {
+ 'parameters': {}
+ }
+ }
+ })");
+ EXPECT_TRUE(manager.LoadBaseCommands(*json, nullptr));
+}
+
+TEST(CommandManager, LoadBaseCommandsFile) {
+ buffet::CommandManager manager;
+ auto json = CreateDictionaryValue(R"({
+ 'base': {
+ 'reboot': {
+ 'parameters': {'delay': 'integer'}
+ },
+ 'shutdown': {
+ 'parameters': {}
+ }
+ }
+ })");
+ base::FilePath temp_file = SaveJsonToTempFile(*json);
+ EXPECT_TRUE(manager.LoadBaseCommands(temp_file, nullptr));
+ base::DeleteFile(temp_file, false);
+}
+
+TEST(CommandManager, LoadCommandsJSON) {
+ buffet::CommandManager manager;
+ auto json = CreateDictionaryValue(R"({
+ 'robot': {
+ '_jump': {
+ 'parameters': {'height': 'integer'}
+ },
+ '_speak': {
+ 'parameters': {'phrase': 'string'}
+ }
+ }
+ })");
+ EXPECT_TRUE(manager.LoadCommands(*json, "category", nullptr));
+}
+
+TEST(CommandManager, LoadCommandsFile) {
+ buffet::CommandManager manager;
+ // Load some standard command definitions first.
+ auto json = CreateDictionaryValue(R"({
+ 'base': {
+ 'reboot': {
+ 'parameters': {'delay': 'integer'}
+ },
+ 'shutdown': {
+ 'parameters': {}
+ }
+ }
+ })");
+ manager.LoadBaseCommands(*json, nullptr);
+ // Load device-supported commands.
+ json = CreateDictionaryValue(R"({
+ 'base': {
+ 'reboot': {
+ 'parameters': {'delay': 'integer'}
+ }
+ },
+ 'robot': {
+ '_jump': {
+ 'parameters': {'height': 'integer'}
+ }
+ }
+ })");
+ base::FilePath temp_file = SaveJsonToTempFile(*json);
+ EXPECT_TRUE(manager.LoadCommands(temp_file, nullptr));
+ base::DeleteFile(temp_file, false);
+ EXPECT_EQ(2, manager.GetCommandDictionary().GetSize());
+ EXPECT_NE(nullptr, manager.GetCommandDictionary().FindCommand("base.reboot"));
+ EXPECT_NE(nullptr, manager.GetCommandDictionary().FindCommand("robot._jump"));
+}
diff --git a/buffet/commands/schema_constants.cc b/buffet/commands/schema_constants.cc
index d1677c0..90c9faa 100644
--- a/buffet/commands/schema_constants.cc
+++ b/buffet/commands/schema_constants.cc
@@ -21,6 +21,7 @@
const char kUnknownProperty[] = "unexpected_parameter";
const char kInvalidObjectSchema[] = "invalid_object_schema";
const char kDuplicateCommandDef[] = "duplicate_command_definition";
+const char kInvalidCommandName[] = "invalid_command_name";
} // namespace commands
} // namespace errors
diff --git a/buffet/commands/schema_constants.h b/buffet/commands/schema_constants.h
index 6a508e0..903228e 100644
--- a/buffet/commands/schema_constants.h
+++ b/buffet/commands/schema_constants.h
@@ -24,6 +24,7 @@
extern const char kUnknownProperty[];
extern const char kInvalidObjectSchema[];
extern const char kDuplicateCommandDef[];
+extern const char kInvalidCommandName[];
} // namespace commands
} // namespace errors
