buffet: Add parsing of command instances from JSON CommandInstance class can be created from a JSON object with proper command and parameter value validation against command definition schema. BUG=chromium:396713 TEST=USE=buffet P2_TEST_FILTER="buffet::*" FEATURES=test emerge-link platform2 Change-Id: Iba4c807225552f6a9d8b33a0aa1fc451e75753a4 Reviewed-on: https://chromium-review.googlesource.com/211338 Reviewed-by: Christopher Wiley <wiley@chromium.org> Commit-Queue: Alex Vakulenko <avakulenko@chromium.org> Tested-by: Alex Vakulenko <avakulenko@chromium.org>
diff --git a/buffet/commands/command_instance.cc b/buffet/commands/command_instance.cc index 7c5e122..1e667fe 100644 --- a/buffet/commands/command_instance.cc +++ b/buffet/commands/command_instance.cc
@@ -4,6 +4,14 @@ #include "buffet/commands/command_instance.h" +#include <base/values.h> + +#include "buffet/commands/command_definition.h" +#include "buffet/commands/command_dictionary.h" +#include "buffet/commands/schema_constants.h" +#include "buffet/commands/schema_utils.h" +#include "buffet/error_codes.h" + namespace buffet { CommandInstance::CommandInstance(const std::string& name, @@ -21,5 +29,91 @@ return value; } +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|. +// On success, returns |true| and the validated parameters and values through +// |parameters|. Otherwise returns |false| and additional error information in +// |error|. +bool GetCommandParameters(const base::DictionaryValue* json, + const CommandDefinition* command_def, + native_types::Object* parameters, + ErrorPtr* error) { + // Get the command parameters from 'parameters' property. + base::DictionaryValue no_params; // Placeholder when no params are specified. + const base::DictionaryValue* params = nullptr; + const base::Value* params_value = nullptr; + if (json->GetWithoutPathExpansion(commands::attributes::kCommand_Parameters, + ¶ms_value)) { + // Make sure the "parameters" property is actually an object. + if (!params_value->GetAsDictionary(¶ms)) { + Error::AddToPrintf(error, errors::json::kDomain, + errors::json::kObjectExpected, + "Property '%s' must be a JSON object", + commands::attributes::kCommand_Parameters); + return false; + } + } else { + // "parameters" are not specified. Assume empty param list. + params = &no_params; + } + + // Now read in the parameters and validate their values against the command + // definition schema. + if (!TypedValueFromJson(params, command_def->GetParameters().get(), + parameters, error)) { + return false; + } + return true; +} + +} // anonymous namespace + +std::unique_ptr<const CommandInstance> CommandInstance::FromJson( + const base::Value* value, + const CommandDictionary& dictionary, + ErrorPtr* error) { + std::unique_ptr<const CommandInstance> instance; + // Get the command JSON object from the value. + const base::DictionaryValue* json = nullptr; + if (!value->GetAsDictionary(&json)) { + Error::AddTo(error, errors::json::kDomain, errors::json::kObjectExpected, + "Command instance is not a JSON object"); + return instance; + } + + // Get the command name from 'name' property. + std::string command_name; + if (!json->GetStringWithoutPathExpansion(commands::attributes::kCommand_Name, + &command_name)) { + Error::AddTo(error, errors::commands::kDomain, + errors::commands::kPropertyMissing, + "Command name is missing"); + return instance; + } + // Make sure we know how to handle the command with this name. + const CommandDefinition* command_def = dictionary.FindCommand(command_name); + if (!command_def) { + Error::AddToPrintf(error, errors::commands::kDomain, + errors::commands::kInvalidCommandName, + "Unknown command received: %s", command_name.c_str()); + return instance; + } + + native_types::Object parameters; + if (!GetCommandParameters(json, command_def, ¶meters, error)) { + Error::AddToPrintf(error, errors::commands::kDomain, + errors::commands::kCommandFailed, + "Failed to validate command '%s'", command_name.c_str()); + return instance; + } + + instance.reset(new CommandInstance(command_name, command_def->GetCategory(), + parameters)); + return instance; +} + } // namespace buffet
diff --git a/buffet/commands/command_instance.h b/buffet/commands/command_instance.h index 0028c88..f5d259d 100644 --- a/buffet/commands/command_instance.h +++ b/buffet/commands/command_instance.h
@@ -13,15 +13,23 @@ #include "buffet/commands/prop_values.h" #include "buffet/commands/schema_utils.h" +#include "buffet/error.h" + +namespace base { +class Value; +} // namespace base namespace buffet { +class CommandDictionary; + class CommandInstance final { public: // Construct a command instance given the full command |name| which must // be in format "<package_name>.<command_name>", a command |category| and // a list of parameters and their values specified in |parameters|. - CommandInstance(const std::string& name, const std::string& category, + CommandInstance(const std::string& name, + const std::string& category, const native_types::Object& parameters); // Returns the full name of the command. @@ -34,6 +42,15 @@ // with given name does not exist, returns null shared_ptr. std::shared_ptr<const PropValue> FindParameter(const std::string& name) const; + // 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 + // fills in error details in |error|. + static std::unique_ptr<const CommandInstance> FromJson( + const base::Value* value, + const CommandDictionary& dictionary, + ErrorPtr* error); + private: // Full command name as "<package_name>.<command_name>". std::string name_;
diff --git a/buffet/commands/command_instance_unittest.cc b/buffet/commands/command_instance_unittest.cc index 0577004..cb639b5 100644 --- a/buffet/commands/command_instance_unittest.cc +++ b/buffet/commands/command_instance_unittest.cc
@@ -4,8 +4,62 @@ #include <gtest/gtest.h> +#include "buffet/commands/command_dictionary.h" #include "buffet/commands/command_instance.h" #include "buffet/commands/prop_types.h" +#include "buffet/commands/unittest_utils.h" + +using buffet::unittests::CreateDictionaryValue; +using buffet::unittests::CreateValue; + +namespace { + +class CommandInstanceTest : public ::testing::Test { + protected: + virtual void SetUp() override { + auto json = CreateDictionaryValue(R"({ + 'base': { + 'reboot': { + 'parameters': {} + } + }, + 'robot': { + 'jump': { + 'parameters': { + 'height': { + 'type': 'integer', + 'minimum': 0, + 'maximum': 100 + }, + '_jumpType': { + 'type': 'string', + 'enum': ['_withAirFlip', '_withSpin', '_withKick'] + } + } + }, + 'speak': { + 'parameters': { + 'phrase': { + 'type': 'string', + 'enum': ['beamMeUpScotty', 'iDontDigOnSwine', + 'iPityDaFool', 'dangerWillRobinson'] + }, + 'volume': { + 'type': 'integer', + 'minimum': 0, + 'maximum': 10 + } + } + } + } + })"); + CHECK(dict_.LoadCommands(*json, "robotd", nullptr, nullptr)) + << "Failed to parse test command dictionary"; + } + buffet::CommandDictionary dict_; +}; + +} // anonymous namespace TEST(CommandInstance, Test) { buffet::native_types::Object params; @@ -23,3 +77,92 @@ EXPECT_EQ(100, instance.FindParameter("volume")->GetInt()->GetValue()); EXPECT_EQ(nullptr, instance.FindParameter("blah").get()); } + +TEST_F(CommandInstanceTest, FromJson) { + auto json = CreateDictionaryValue(R"({ + 'name': 'robot.jump', + 'parameters': { + 'height': 53, + '_jumpType': '_withKick' + } + })"); + auto instance = buffet::CommandInstance::FromJson(json.get(), dict_, nullptr); + EXPECT_EQ("robot.jump", instance->GetName()); + EXPECT_EQ("robotd", instance->GetCategory()); + EXPECT_EQ(53, instance->FindParameter("height")->GetInt()->GetValue()); + EXPECT_EQ("_withKick", + instance->FindParameter("_jumpType")->GetString()->GetValue()); +} + +TEST_F(CommandInstanceTest, FromJson_ParamsOmitted) { + auto json = CreateDictionaryValue("{'name': 'base.reboot'}"); + auto instance = buffet::CommandInstance::FromJson(json.get(), dict_, nullptr); + EXPECT_EQ("base.reboot", instance->GetName()); + EXPECT_EQ("robotd", instance->GetCategory()); + EXPECT_TRUE(instance->GetParameters().empty()); +} + +TEST_F(CommandInstanceTest, FromJson_NotObject) { + auto json = CreateValue("'string'"); + buffet::ErrorPtr error; + auto instance = buffet::CommandInstance::FromJson(json.get(), dict_, &error); + EXPECT_EQ(nullptr, instance.get()); + EXPECT_EQ("json_object_expected", error->GetCode()); + EXPECT_EQ("Command instance is not a JSON object", error->GetMessage()); +} + +TEST_F(CommandInstanceTest, FromJson_NameMissing) { + auto json = CreateDictionaryValue("{'param': 'value'}"); + buffet::ErrorPtr error; + auto instance = buffet::CommandInstance::FromJson(json.get(), dict_, &error); + EXPECT_EQ(nullptr, instance.get()); + EXPECT_EQ("parameter_missing", error->GetCode()); + EXPECT_EQ("Command name is missing", error->GetMessage()); +} + +TEST_F(CommandInstanceTest, FromJson_UnknownCommand) { + auto json = CreateDictionaryValue("{'name': 'robot.scream'}"); + buffet::ErrorPtr error; + auto instance = buffet::CommandInstance::FromJson(json.get(), dict_, &error); + EXPECT_EQ(nullptr, instance.get()); + EXPECT_EQ("invalid_command_name", error->GetCode()); + EXPECT_EQ("Unknown command received: robot.scream", error->GetMessage()); +} + +TEST_F(CommandInstanceTest, FromJson_ParamsNotObject) { + auto json = CreateDictionaryValue(R"({ + 'name': 'robot.speak', + 'parameters': 'hello' + })"); + buffet::ErrorPtr error; + auto instance = buffet::CommandInstance::FromJson(json.get(), dict_, &error); + EXPECT_EQ(nullptr, instance.get()); + auto inner = error->GetInnerError(); + EXPECT_EQ("json_object_expected", inner->GetCode()); + EXPECT_EQ("Property 'parameters' must be a JSON object", inner->GetMessage()); + EXPECT_EQ("command_failed", error->GetCode()); + EXPECT_EQ("Failed to validate command 'robot.speak'", error->GetMessage()); +} + +TEST_F(CommandInstanceTest, FromJson_ParamError) { + auto json = CreateDictionaryValue(R"({ + 'name': 'robot.speak', + 'parameters': { + 'phrase': 'iPityDaFool', + 'volume': 20 + } + })"); + buffet::ErrorPtr error; + auto instance = buffet::CommandInstance::FromJson(json.get(), dict_, &error); + EXPECT_EQ(nullptr, instance.get()); + auto first = error->GetFirstError(); + EXPECT_EQ("out_of_range", first->GetCode()); + EXPECT_EQ("Value 20 is out of range. It must not be greater than 10", + first->GetMessage()); + auto inner = error->GetInnerError(); + EXPECT_EQ("invalid_parameter_value", inner->GetCode()); + EXPECT_EQ("Invalid parameter value for property 'volume'", + inner->GetMessage()); + EXPECT_EQ("command_failed", error->GetCode()); + EXPECT_EQ("Failed to validate command 'robot.speak'", error->GetMessage()); +}
diff --git a/buffet/commands/command_queue.h b/buffet/commands/command_queue.h index 5d650a0..3f269f1 100644 --- a/buffet/commands/command_queue.h +++ b/buffet/commands/command_queue.h
@@ -26,8 +26,10 @@ size_t GetCount() const { return map_.size(); } // Adds a new command to the queue. Each command in the queue has a unique - // ID that identifies that command instance. The ID string of the added - // command is returned by this method. + // ID that identifies that command instance in this queue. This identifier + // has no relation to any GCD command identifiers or anything else. Just a + // unique key in this queue class. + // The ID string of the added command is returned by this method. std::string Add(std::unique_ptr<const CommandInstance> instance); // Removes a command identified by |id| from the queue. Returns a unique
diff --git a/buffet/commands/schema_constants.cc b/buffet/commands/schema_constants.cc index 90c9faa..74f96af 100644 --- a/buffet/commands/schema_constants.cc +++ b/buffet/commands/schema_constants.cc
@@ -22,6 +22,7 @@ const char kInvalidObjectSchema[] = "invalid_object_schema"; const char kDuplicateCommandDef[] = "duplicate_command_definition"; const char kInvalidCommandName[] = "invalid_command_name"; +const char kCommandFailed[] = "command_failed"; } // namespace commands } // namespace errors @@ -42,6 +43,7 @@ const char kObject_Properties[] = "properties"; +const char kCommand_Name[] = "name"; const char kCommand_Parameters[] = "parameters"; } // namespace attributes } // namespace commands
diff --git a/buffet/commands/schema_constants.h b/buffet/commands/schema_constants.h index 903228e..2375ada 100644 --- a/buffet/commands/schema_constants.h +++ b/buffet/commands/schema_constants.h
@@ -25,6 +25,7 @@ extern const char kInvalidObjectSchema[]; extern const char kDuplicateCommandDef[]; extern const char kInvalidCommandName[]; +extern const char kCommandFailed[]; } // namespace commands } // namespace errors @@ -46,6 +47,7 @@ extern const char kObject_Properties[]; +extern const char kCommand_Name[]; extern const char kCommand_Parameters[]; } // namespace attributes } // namespace commands