buffet: Provide results definition in command definition.
That allows us to provide typed results for supported commands.
BUG=chromium:435607
TEST=cros_workon_make --test buffet
Change-Id: I61b5a5b294b4d869366c821adf1ef7f6db31c7ea
Reviewed-on: https://chromium-review.googlesource.com/231322
Reviewed-by: Anton Muhin <antonm@chromium.org>
Commit-Queue: Anton Muhin <antonm@chromium.org>
Tested-by: Anton Muhin <antonm@chromium.org>
diff --git a/buffet/commands/command_definition.cc b/buffet/commands/command_definition.cc
index f24b22d..7494de8 100644
--- a/buffet/commands/command_definition.cc
+++ b/buffet/commands/command_definition.cc
@@ -8,8 +8,9 @@
CommandDefinition::CommandDefinition(
const std::string& category,
- const std::shared_ptr<const ObjectSchema>& parameters)
- : category_(category), parameters_(parameters) {
+ const std::shared_ptr<const ObjectSchema>& parameters,
+ const std::shared_ptr<const ObjectSchema>& results)
+ : category_(category), parameters_(parameters), results_(results) {
}
} // namespace buffet
diff --git a/buffet/commands/command_definition.h b/buffet/commands/command_definition.h
index f02cf48..a7b793a 100644
--- a/buffet/commands/command_definition.h
+++ b/buffet/commands/command_definition.h
@@ -22,7 +22,8 @@
class CommandDefinition {
public:
CommandDefinition(const std::string& category,
- const std::shared_ptr<const ObjectSchema>& parameters);
+ const std::shared_ptr<const ObjectSchema>& parameters,
+ const std::shared_ptr<const ObjectSchema>& results);
// Gets the category this command belongs to.
const std::string& GetCategory() const { return category_; }
@@ -30,10 +31,15 @@
const std::shared_ptr<const ObjectSchema>& GetParameters() const {
return parameters_;
}
+ // Gets the object schema for command results.
+ const std::shared_ptr<const ObjectSchema>& GetResults() const {
+ return results_;
+ }
private:
std::string category_; // Cmd category. Could be "powerd" for "base.reboot".
- std::shared_ptr<const ObjectSchema> parameters_; // Command parameter def.
+ std::shared_ptr<const ObjectSchema> parameters_; // Command parameters def.
+ std::shared_ptr<const ObjectSchema> results_; // Command results def.
DISALLOW_COPY_AND_ASSIGN(CommandDefinition);
};
diff --git a/buffet/commands/command_definition_unittest.cc b/buffet/commands/command_definition_unittest.cc
index f0adab1..0b06086 100644
--- a/buffet/commands/command_definition_unittest.cc
+++ b/buffet/commands/command_definition_unittest.cc
@@ -8,7 +8,9 @@
TEST(CommandDefinition, Test) {
auto params = std::make_shared<buffet::ObjectSchema>();
- buffet::CommandDefinition def("powerd", params);
+ auto results = std::make_shared<buffet::ObjectSchema>();
+ buffet::CommandDefinition def("powerd", params, results);
EXPECT_EQ("powerd", def.GetCategory());
EXPECT_EQ(params, def.GetParameters());
+ EXPECT_EQ(results, def.GetResults());
}
diff --git a/buffet/commands/command_dictionary.cc b/buffet/commands/command_dictionary.cc
index 98ffefc..930e044 100644
--- a/buffet/commands/command_dictionary.cc
+++ b/buffet/commands/command_dictionary.cc
@@ -54,8 +54,8 @@
package_name.c_str());
return false;
}
- const base::DictionaryValue* command_value = nullptr;
- if (!command_iter.value().GetAsDictionary(&command_value)) {
+ const base::DictionaryValue* command_def_json = nullptr;
+ if (!command_iter.value().GetAsDictionary(&command_def_json)) {
chromeos::Error::AddToPrintf(error, FROM_HERE,
errors::commands::kDomain,
errors::commands::kTypeMismatch,
@@ -66,32 +66,22 @@
// Construct the compound command name as "pkg_name.cmd_name".
std::string full_command_name = chromeos::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;
- if (!command_value->GetDictionaryWithoutPathExpansion(
- commands::attributes::kCommand_Parameters, &command_schema_def)) {
- chromeos::Error::AddToPrintf(
- error, FROM_HERE, errors::commands::kDomain,
- errors::commands::kPropertyMissing,
- "Command definition '%s' is missing property '%s'",
- full_command_name.c_str(),
- commands::attributes::kCommand_Parameters);
- return false;
- }
- const ObjectSchema* base_def = nullptr;
+ const ObjectSchema* base_parameters_def = nullptr;
+ const ObjectSchema* base_results_def = nullptr;
if (base_commands) {
const CommandDefinition* cmd =
base_commands->FindCommand(full_command_name);
- if (cmd)
- base_def = cmd->GetParameters().get();
+ if (cmd) {
+ base_parameters_def = cmd->GetParameters().get();
+ base_results_def = cmd->GetResults().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 (!cmd) {
if (command_name.front() != '_') {
chromeos::Error::AddToPrintf(
error, FROM_HERE, errors::commands::kDomain,
@@ -104,17 +94,27 @@
}
}
- auto command_schema = std::make_shared<ObjectSchema>();
- if (!command_schema->FromJson(command_schema_def, base_def, error)) {
- chromeos::Error::AddToPrintf(error, FROM_HERE,
- errors::commands::kDomain,
- errors::commands::kInvalidObjectSchema,
- "Invalid definition for command '%s'",
- full_command_name.c_str());
+ auto parameters_schema = BuildObjectSchema(
+ command_def_json,
+ commands::attributes::kCommand_Parameters,
+ base_parameters_def,
+ full_command_name,
+ error);
+ if (!parameters_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;
+
auto command_def = std::make_shared<CommandDefinition>(category,
- command_schema);
+ parameters_schema,
+ results_schema);
new_defs.insert(std::make_pair(full_command_name, command_def));
command_iter.Advance();
@@ -150,6 +150,38 @@
return true;
}
+std::shared_ptr<ObjectSchema> CommandDictionary::BuildObjectSchema(
+ const base::DictionaryValue* command_def_json,
+ const char* property_name,
+ const ObjectSchema* base_def,
+ const std::string& command_name,
+ chromeos::ErrorPtr* error) {
+ auto object_schema = std::make_shared<ObjectSchema>();
+
+ const base::DictionaryValue* schema_def = nullptr;
+ if (!command_def_json->GetDictionaryWithoutPathExpansion(property_name,
+ &schema_def)) {
+ chromeos::Error::AddToPrintf(
+ error, FROM_HERE, errors::commands::kDomain,
+ errors::commands::kPropertyMissing,
+ "Command definition '%s' is missing property '%s'",
+ command_name.c_str(),
+ property_name);
+ return {};
+ }
+
+ if (!object_schema->FromJson(schema_def, base_def, error)) {
+ chromeos::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(
bool full_schema, chromeos::ErrorPtr* error) const {
std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue);
diff --git a/buffet/commands/command_dictionary.h b/buffet/commands/command_dictionary.h
index bd60986..c316fd5 100644
--- a/buffet/commands/command_dictionary.h
+++ b/buffet/commands/command_dictionary.h
@@ -21,6 +21,7 @@
namespace buffet {
class CommandDefinition;
+class ObjectSchema;
// CommandDictionary is a wrapper around a map of command name and the
// corresponding command definition schema. The command name (the key in
@@ -71,6 +72,13 @@
const CommandDefinition* FindCommand(const std::string& command_name) const;
private:
+ std::shared_ptr<ObjectSchema> BuildObjectSchema(
+ const base::DictionaryValue* command_def_json,
+ const char* property_name,
+ const ObjectSchema* base_def,
+ const std::string& command_name,
+ chromeos::ErrorPtr* error);
+
using CommandMap = std::map<std::string,
std::shared_ptr<const CommandDefinition>>;
diff --git a/buffet/commands/command_dictionary_unittest.cc b/buffet/commands/command_dictionary_unittest.cc
index 20c1287..10e7ec9 100644
--- a/buffet/commands/command_dictionary_unittest.cc
+++ b/buffet/commands/command_dictionary_unittest.cc
@@ -24,7 +24,8 @@
'parameters': {
'height': 'integer',
'_jumpType': ['_withAirFlip', '_withSpin', '_withKick']
- }
+ },
+ 'results': {}
}
}
})");
@@ -35,10 +36,12 @@
json = CreateDictionaryValue(R"({
'base': {
'reboot': {
- 'parameters': {'delay': 'integer'}
+ 'parameters': {'delay': 'integer'},
+ 'results': {}
},
'shutdown': {
- 'parameters': {}
+ 'parameters': {},
+ 'results': {}
}
}
})");
@@ -57,13 +60,21 @@
chromeos::ErrorPtr error;
// Command definition missing 'parameters' property.
- auto json = CreateDictionaryValue("{'robot':{'jump':{}}}");
+ auto json = CreateDictionaryValue("{'robot':{'jump':{'results':{}}}}");
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());
error.reset();
+ // Command definition missing 'results' property.
+ json = CreateDictionaryValue("{'robot':{'jump':{'parameters':{}}}}");
+ EXPECT_FALSE(dict.LoadCommands(*json, "robotd", nullptr, &error));
+ EXPECT_EQ("parameter_missing", error->GetCode());
+ EXPECT_EQ("Command definition 'robot.jump' is missing property 'results'",
+ error->GetMessage());
+ error.reset();
+
// Command definition is not an object.
json = CreateDictionaryValue("{'robot':{'jump':0}}");
EXPECT_FALSE(dict.LoadCommands(*json, "robotd", nullptr, &error));
@@ -79,7 +90,8 @@
error.reset();
// Invalid command definition is not an object.
- json = CreateDictionaryValue("{'robot':{'jump':{'parameters':{'flip':0}}}}");
+ json = CreateDictionaryValue(
+ "{'robot':{'jump':{'parameters':{'flip':0},'results':{}}}}");
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());
@@ -87,7 +99,7 @@
error.reset();
// Empty command name.
- json = CreateDictionaryValue("{'robot':{'':{'parameters':{'flip':0}}}}");
+ json = CreateDictionaryValue("{'robot':{'':{'parameters':{},'results':{}}}}");
EXPECT_FALSE(dict.LoadCommands(*json, "robotd", nullptr, &error));
EXPECT_EQ("invalid_command_name", error->GetCode());
EXPECT_EQ("Unnamed command encountered in package 'robot'",
@@ -99,7 +111,8 @@
// Redefine commands in different category.
buffet::CommandDictionary dict;
chromeos::ErrorPtr error;
- auto json = CreateDictionaryValue("{'robot':{'jump':{'parameters':{}}}}");
+ auto json = CreateDictionaryValue(
+ "{'robot':{'jump':{'parameters':{},'results':{}}}}");
dict.LoadCommands(*json, "category1", nullptr, &error);
EXPECT_FALSE(dict.LoadCommands(*json, "category2", nullptr, &error));
EXPECT_EQ("duplicate_command_definition", error->GetCode());
@@ -116,13 +129,15 @@
auto json = CreateDictionaryValue(R"({
'base': {
'reboot': {
- 'parameters': {'delay': 'integer'}
+ 'parameters': {'delay': 'integer'},
+ 'results': {}
}
}
})");
base_dict.LoadCommands(*json, "", nullptr, &error);
EXPECT_TRUE(dict.LoadCommands(*json, "robotd", &base_dict, &error));
- auto json2 = CreateDictionaryValue("{'base':{'jump':{'parameters':{}}}}");
+ auto json2 = CreateDictionaryValue(
+ "{'base':{'jump':{'parameters':{},'results':{}}}}");
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 "
@@ -130,7 +145,8 @@
error.reset();
// If the command starts with "_", then it's Ok.
- json2 = CreateDictionaryValue("{'base':{'_jump':{'parameters':{}}}}");
+ json2 = CreateDictionaryValue(
+ "{'base':{'_jump':{'parameters':{},'results':{}}}}");
EXPECT_TRUE(dict.LoadCommands(*json2, "robotd", &base_dict, nullptr));
}
@@ -142,15 +158,18 @@
auto json = CreateDictionaryValue(R"({
'base': {
'reboot': {
- 'parameters': {'delay': 'integer'}
+ 'parameters': {'delay': 'integer'},
+ 'results': {'version': 'integer'}
}
}
})");
base_dict.LoadCommands(*json, "", nullptr, &error);
+
auto json2 = CreateDictionaryValue(R"({
'base': {
'reboot': {
- 'parameters': {'delay': 'string'}
+ 'parameters': {'delay': 'string'},
+ 'results': {'version': 'integer'}
}
}
})");
@@ -165,16 +184,39 @@
EXPECT_EQ("Redefining a property of type integer as string",
error->GetFirstError()->GetMessage());
error.reset();
+
+ auto json3 = CreateDictionaryValue(R"({
+ 'base': {
+ 'reboot': {
+ 'parameters': {'delay': 'integer'},
+ 'results': {'version': 'string'}
+ }
+ }
+ })");
+ EXPECT_FALSE(dict.LoadCommands(*json3, "robotd", &base_dict, &error));
+ EXPECT_EQ("invalid_object_schema", error->GetCode());
+ EXPECT_EQ("Invalid definition for command 'base.reboot'",
+ error->GetMessage());
+ // TODO(antonm): remove parameter from error below and use some generic.
+ EXPECT_EQ("invalid_parameter_definition", error->GetInnerError()->GetCode());
+ EXPECT_EQ("Error in definition of property 'version'",
+ 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();
}
TEST(CommandDictionary, GetCommandsAsJson) {
auto json_base = CreateDictionaryValue(R"({
'base': {
'reboot': {
- 'parameters': {'delay': {'maximum': 100}}
+ 'parameters': {'delay': {'maximum': 100}},
+ 'results': {}
},
'shutdown': {
- 'parameters': {}
+ 'parameters': {},
+ 'results': {}
}
}
})");
@@ -184,12 +226,14 @@
auto json = buffet::unittests::CreateDictionaryValue(R"({
'base': {
'reboot': {
- 'parameters': {'delay': {'minimum': 10}}
+ 'parameters': {'delay': {'minimum': 10}},
+ 'results': {}
}
},
'robot': {
'_jump': {
- 'parameters': {'_height': 'integer'}
+ 'parameters': {'_height': 'integer'},
+ 'results': {}
}
}
})");
diff --git a/buffet/commands/command_instance_unittest.cc b/buffet/commands/command_instance_unittest.cc
index b57de25..d5afc70 100644
--- a/buffet/commands/command_instance_unittest.cc
+++ b/buffet/commands/command_instance_unittest.cc
@@ -21,7 +21,8 @@
auto json = CreateDictionaryValue(R"({
'base': {
'reboot': {
- 'parameters': {}
+ 'parameters': {},
+ 'results': {}
}
},
'robot': {
@@ -36,7 +37,8 @@
'type': 'string',
'enum': ['_withAirFlip', '_withSpin', '_withKick']
}
- }
+ },
+ 'results': {}
},
'speak': {
'parameters': {
@@ -50,7 +52,8 @@
'minimum': 0,
'maximum': 10
}
- }
+ },
+ 'results': {}
}
}
})");
@@ -92,7 +95,8 @@
'parameters': {
'height': 53,
'_jumpType': '_withKick'
- }
+ },
+ 'results': {}
})");
auto instance = buffet::CommandInstance::FromJson(json.get(), dict_, nullptr);
EXPECT_EQ("robot.jump", instance->GetName());
diff --git a/buffet/commands/command_manager_unittest.cc b/buffet/commands/command_manager_unittest.cc
index 37e3716..c40f2dc 100644
--- a/buffet/commands/command_manager_unittest.cc
+++ b/buffet/commands/command_manager_unittest.cc
@@ -31,10 +31,12 @@
auto json = CreateDictionaryValue(R"({
'base': {
'reboot': {
- 'parameters': {'delay': 'integer'}
+ 'parameters': {'delay': 'integer'},
+ 'results': {}
},
'shutdown': {
- 'parameters': {}
+ 'parameters': {},
+ 'results': {}
}
}
})");
@@ -46,10 +48,12 @@
auto json = CreateDictionaryValue(R"({
'base': {
'reboot': {
- 'parameters': {'delay': 'integer'}
+ 'parameters': {'delay': 'integer'},
+ 'results': {}
},
'shutdown': {
- 'parameters': {}
+ 'parameters': {},
+ 'results': {}
}
}
})");
@@ -63,10 +67,12 @@
auto json = CreateDictionaryValue(R"({
'robot': {
'_jump': {
- 'parameters': {'height': 'integer'}
+ 'parameters': {'height': 'integer'},
+ 'results': {}
},
'_speak': {
- 'parameters': {'phrase': 'string'}
+ 'parameters': {'phrase': 'string'},
+ 'results': {}
}
}
})");
@@ -79,10 +85,12 @@
auto json = CreateDictionaryValue(R"({
'base': {
'reboot': {
- 'parameters': {'delay': 'integer'}
+ 'parameters': {'delay': 'integer'},
+ 'results': {}
},
'shutdown': {
- 'parameters': {}
+ 'parameters': {},
+ 'results': {}
}
}
})");
@@ -91,12 +99,14 @@
json = CreateDictionaryValue(R"({
'base': {
'reboot': {
- 'parameters': {'delay': 'integer'}
+ 'parameters': {'delay': 'integer'},
+ 'results': {}
}
},
'robot': {
'_jump': {
- 'parameters': {'height': 'integer'}
+ 'parameters': {'height': 'integer'},
+ 'results': {}
}
}
})");
diff --git a/buffet/commands/dbus_command_dispatcher_unittest.cc b/buffet/commands/dbus_command_dispatcher_unittest.cc
index af15c1d..ed272a1 100644
--- a/buffet/commands/dbus_command_dispatcher_unittest.cc
+++ b/buffet/commands/dbus_command_dispatcher_unittest.cc
@@ -73,10 +73,12 @@
auto json = CreateDictionaryValue(R"({
'base': {
'reboot': {
- 'parameters': {'delay': 'integer'}
+ 'parameters': {'delay': 'integer'},
+ 'results': {}
},
'shutdown': {
- 'parameters': {}
+ 'parameters': {},
+ 'results': {}
}
}
})");
diff --git a/buffet/commands/dbus_command_proxy_unittest.cc b/buffet/commands/dbus_command_proxy_unittest.cc
index 3cb907d..81d9827 100644
--- a/buffet/commands/dbus_command_proxy_unittest.cc
+++ b/buffet/commands/dbus_command_proxy_unittest.cc
@@ -50,6 +50,7 @@
EXPECT_CALL(*bus_, AssertOnDBusThread()).Times(AnyNumber());
// Command instance.
+ // TODO(antonm): Test results.
auto json = CreateDictionaryValue(R"({
'robot': {
'jump': {
@@ -63,7 +64,8 @@
'type': 'string',
'enum': ['_withAirFlip', '_withSpin', '_withKick']
}
- }
+ },
+ 'results': {}
}
}
})");
diff --git a/buffet/commands/schema_constants.cc b/buffet/commands/schema_constants.cc
index faf58fb..a26c355 100644
--- a/buffet/commands/schema_constants.cc
+++ b/buffet/commands/schema_constants.cc
@@ -46,6 +46,7 @@
const char kCommand_Id[] = "id";
const char kCommand_Name[] = "name";
const char kCommand_Parameters[] = "parameters";
+const char kCommand_Results[] = "results";
} // namespace attributes
} // namespace commands
diff --git a/buffet/commands/schema_constants.h b/buffet/commands/schema_constants.h
index d5a8c4e..6b06e2f 100644
--- a/buffet/commands/schema_constants.h
+++ b/buffet/commands/schema_constants.h
@@ -50,6 +50,7 @@
extern const char kCommand_Id[];
extern const char kCommand_Name[];
extern const char kCommand_Parameters[];
+extern const char kCommand_Results[];
} // namespace attributes
} // namespace commands
diff --git a/buffet/device_registration_info_unittest.cc b/buffet/device_registration_info_unittest.cc
index 1ab5191..dd8ffbe 100644
--- a/buffet/device_registration_info_unittest.cc
+++ b/buffet/device_registration_info_unittest.cc
@@ -316,10 +316,12 @@
auto json_base = buffet::unittests::CreateDictionaryValue(R"({
'base': {
'reboot': {
- 'parameters': {'delay': 'integer'}
+ 'parameters': {'delay': 'integer'},
+ 'results': {}
},
'shutdown': {
- 'parameters': {}
+ 'parameters': {},
+ 'results': {}
}
}
})");
@@ -327,12 +329,14 @@
auto json_cmds = buffet::unittests::CreateDictionaryValue(R"({
'base': {
'reboot': {
- 'parameters': {'delay': {'minimum': 10}}
+ 'parameters': {'delay': {'minimum': 10}},
+ 'results': {}
}
},
'robot': {
'_jump': {
- 'parameters': {'_height': 'integer'}
+ 'parameters': {'_height': 'integer'},
+ 'results': {}
}
}
})");
diff --git a/buffet/etc/buffet/commands/test.json b/buffet/etc/buffet/commands/test.json
index 10f8297..f6201c7 100644
--- a/buffet/etc/buffet/commands/test.json
+++ b/buffet/etc/buffet/commands/test.json
@@ -2,11 +2,15 @@
"base": {
"reboot": {
"parameters": {
+ },
+ "results": {
}
},
"_jump": {
"parameters": {
"_height":"integer"
+ },
+ "results": {
}
}
}
diff --git a/buffet/etc/buffet/gcd.json b/buffet/etc/buffet/gcd.json
index b488802..1f37cec 100644
--- a/buffet/etc/buffet/gcd.json
+++ b/buffet/etc/buffet/gcd.json
@@ -2,6 +2,8 @@
"base": {
"reboot": {
"parameters": {
+ },
+ "results": {
}
}
}
