buffet: Add support for "required" and "isRequired" parameters Added support for "required" attribute for object types and "isRequired" for command parameters and made all properties/parameters optional by default. BUG=brillo:354 TEST=`FEATURES=test emerge-link buffet` Change-Id: Ie3c7607e4ac0319f8ed459875a823fed39890da9 Reviewed-on: https://chromium-review.googlesource.com/283646 Trybot-Ready: Alex Vakulenko <avakulenko@chromium.org> Tested-by: Alex Vakulenko <avakulenko@chromium.org> Reviewed-by: Vitaly Buka <vitalybuka@chromium.org> Commit-Queue: Alex Vakulenko <avakulenko@chromium.org>
diff --git a/buffet/etc/buffet/commands/test.json b/buffet/etc/buffet/commands/test.json index aac439d..911d8f0 100644 --- a/buffet/etc/buffet/commands/test.json +++ b/buffet/etc/buffet/commands/test.json
@@ -2,13 +2,14 @@ "base": { "_jump": { "parameters": { - "_height":"integer" + "_height": "integer" }, "progress": { "progress": { "type": "integer", "minimum": 0, - "maximum": 100 + "maximum": 100, + "isRequired": true } } }
diff --git a/buffet/etc/buffet/gcd.json b/buffet/etc/buffet/gcd.json index ac85f6b..368704e 100644 --- a/buffet/etc/buffet/gcd.json +++ b/buffet/etc/buffet/gcd.json
@@ -26,7 +26,8 @@ "minimalRole": "manager", "parameters": { "description": "string", - "name": "string" + "name": "string", + "location": "string" }, "results": {} }
diff --git a/libweave/src/base_api_handler_unittest.cc b/libweave/src/base_api_handler_unittest.cc index a79abe0..c7456cf 100644 --- a/libweave/src/base_api_handler_unittest.cc +++ b/libweave/src/base_api_handler_unittest.cc
@@ -179,6 +179,17 @@ EXPECT_EQ("testName", config.name()); EXPECT_EQ("testDescription", config.description()); EXPECT_EQ("testLocation", config.location()); + + AddCommand(R"({ + 'name' : 'base.updateDeviceInfo', + 'parameters': { + 'location': 'newLocation' + } + })"); + + EXPECT_EQ("testName", config.name()); + EXPECT_EQ("testDescription", config.description()); + EXPECT_EQ("newLocation", config.location()); } } // namespace buffet
diff --git a/libweave/src/commands/command_dictionary.cc b/libweave/src/commands/command_dictionary.cc index 92dd316..69932d7 100644 --- a/libweave/src/commands/command_dictionary.cc +++ b/libweave/src/commands/command_dictionary.cc
@@ -217,7 +217,7 @@ continue; std::unique_ptr<base::DictionaryValue> parameters = - pair.second->GetParameters()->ToJson(full_schema, error); + pair.second->GetParameters()->ToJson(full_schema, true, error); if (!parameters) return {}; // Progress and results are not part of public commandDefs.
diff --git a/libweave/src/commands/command_dictionary_unittest.cc b/libweave/src/commands/command_dictionary_unittest.cc index e087943..278364f 100644 --- a/libweave/src/commands/command_dictionary_unittest.cc +++ b/libweave/src/commands/command_dictionary_unittest.cc
@@ -92,11 +92,11 @@ EXPECT_EQ(UserRole::kViewer, cmd->GetMinimalRole()); EXPECT_JSON_EQ("{'height': {'type': 'integer'}}", - *cmd->GetParameters()->ToJson(true, nullptr)); + *cmd->GetParameters()->ToJson(true, true, nullptr)); EXPECT_JSON_EQ("{'progress': {'type': 'integer'}}", - *cmd->GetProgress()->ToJson(true, nullptr)); + *cmd->GetProgress()->ToJson(true, false, nullptr)); EXPECT_JSON_EQ("{'success': {'type': 'boolean'}}", - *cmd->GetResults()->ToJson(true, nullptr)); + *cmd->GetResults()->ToJson(true, false, nullptr)); } TEST(CommandDictionary, LoadCommands_Failures) {
diff --git a/libweave/src/commands/object_schema.cc b/libweave/src/commands/object_schema.cc index 2fec07b..27ace1c 100644 --- a/libweave/src/commands/object_schema.cc +++ b/libweave/src/commands/object_schema.cc
@@ -287,12 +287,25 @@ return p != properties_.end() ? p->second.get() : nullptr; } +bool ObjectSchema::MarkPropRequired(const std::string& name, + chromeos::ErrorPtr* error) { + auto p = properties_.find(name); + if (p == properties_.end()) { + chromeos::Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain, + errors::commands::kUnknownProperty, + "Unknown property '%s'", name.c_str()); + return false; + } + p->second->MakeRequired(true); + return true; +} std::unique_ptr<base::DictionaryValue> ObjectSchema::ToJson( bool full_schema, + bool in_command_def, chromeos::ErrorPtr* error) const { std::unique_ptr<base::DictionaryValue> value(new base::DictionaryValue); for (const auto& pair : properties_) { - auto PropDef = pair.second->ToJson(full_schema, error); + auto PropDef = pair.second->ToJson(full_schema, in_command_def, error); if (!PropDef) return {}; value->SetWithoutPathExpansion(pair.first, PropDef.release());
diff --git a/libweave/src/commands/object_schema.h b/libweave/src/commands/object_schema.h index d6eafed..577a5d4 100644 --- a/libweave/src/commands/object_schema.h +++ b/libweave/src/commands/object_schema.h
@@ -49,6 +49,11 @@ // Gets the list of all the properties defined. const Properties& GetProps() const { return properties_; } + // Marks the property with given name as "required". If |name| specifies + // an unknown property, false is returned and |error| is set with detailed + // error message for the failure. + bool MarkPropRequired(const std::string& name, chromeos::ErrorPtr* error); + // Specify whether extra properties are allowed on objects described by // this schema. When validating a value of an object type, we can // make sure that the value has only the properties explicitly defined by @@ -64,6 +69,7 @@ // the overridden (not inherited) ones are saved. std::unique_ptr<base::DictionaryValue> ToJson( bool full_schema, + bool in_command_def, chromeos::ErrorPtr* error) const; // Loads the object schema from JSON. If |object_schema| is not nullptr, it is
diff --git a/libweave/src/commands/object_schema_unittest.cc b/libweave/src/commands/object_schema_unittest.cc index 12bf8a5..b05e315 100644 --- a/libweave/src/commands/object_schema_unittest.cc +++ b/libweave/src/commands/object_schema_unittest.cc
@@ -68,25 +68,26 @@ TEST(CommandSchema, IntPropType_ToJson) { IntPropType prop; - EXPECT_JSON_EQ("'integer'", *prop.ToJson(false, nullptr)); - EXPECT_JSON_EQ("{'type':'integer'}", *prop.ToJson(true, nullptr)); + EXPECT_JSON_EQ("'integer'", *prop.ToJson(false, false, nullptr)); + EXPECT_JSON_EQ("{'type':'integer'}", *prop.ToJson(true, false, nullptr)); IntPropType param2; param2.FromJson(CreateDictionaryValue("{}").get(), &prop, nullptr); - EXPECT_JSON_EQ("{}", *param2.ToJson(false, nullptr)); + EXPECT_JSON_EQ("{}", *param2.ToJson(false, false, nullptr)); param2.FromJson(CreateDictionaryValue("{'minimum':3}").get(), &prop, nullptr); - EXPECT_JSON_EQ("{'minimum':3}", *param2.ToJson(false, nullptr)); + EXPECT_JSON_EQ("{'minimum':3}", *param2.ToJson(false, false, nullptr)); param2.FromJson(CreateDictionaryValue("{'maximum':-7}").get(), &prop, nullptr); - EXPECT_JSON_EQ("{'maximum':-7}", *param2.ToJson(false, nullptr)); + EXPECT_JSON_EQ("{'maximum':-7}", *param2.ToJson(false, false, nullptr)); param2.FromJson(CreateDictionaryValue("{'minimum':0,'maximum':5}").get(), &prop, nullptr); - EXPECT_JSON_EQ("{'maximum':5,'minimum':0}", *param2.ToJson(false, nullptr)); + EXPECT_JSON_EQ("{'maximum':5,'minimum':0}", + *param2.ToJson(false, false, nullptr)); param2.FromJson(CreateDictionaryValue("{'enum':[1,2,3]}").get(), &prop, nullptr); - EXPECT_JSON_EQ("[1,2,3]", *param2.ToJson(false, nullptr)); + EXPECT_JSON_EQ("[1,2,3]", *param2.ToJson(false, false, nullptr)); param2.FromJson(CreateDictionaryValue("{'default':123}").get(), &prop, nullptr); - EXPECT_JSON_EQ("{'default':123}", *param2.ToJson(false, nullptr)); + EXPECT_JSON_EQ("{'default':123}", *param2.ToJson(false, false, nullptr)); } TEST(CommandSchema, IntPropType_FromJson) { @@ -172,6 +173,7 @@ EXPECT_FALSE(prop.HasOverriddenAttributes()); EXPECT_FALSE(prop.IsBasedOnSchema()); EXPECT_EQ(nullptr, prop.GetDefaultValue()); + EXPECT_FALSE(prop.IsRequired()); } TEST(CommandSchema, BoolPropType_Types) { @@ -186,19 +188,19 @@ TEST(CommandSchema, BoolPropType_ToJson) { BooleanPropType prop; - EXPECT_JSON_EQ("'boolean'", *prop.ToJson(false, nullptr)); - EXPECT_JSON_EQ("{'type':'boolean'}", *prop.ToJson(true, nullptr)); + EXPECT_JSON_EQ("'boolean'", *prop.ToJson(false, false, nullptr)); + EXPECT_JSON_EQ("{'type':'boolean'}", *prop.ToJson(true, false, nullptr)); BooleanPropType param2; param2.FromJson(CreateDictionaryValue("{}").get(), &prop, nullptr); - EXPECT_JSON_EQ("{}", *param2.ToJson(false, nullptr)); + EXPECT_JSON_EQ("{}", *param2.ToJson(false, false, nullptr)); param2.FromJson(CreateDictionaryValue("{'enum':[true,false]}").get(), &prop, nullptr); - EXPECT_JSON_EQ("[true,false]", *param2.ToJson(false, nullptr)); + EXPECT_JSON_EQ("[true,false]", *param2.ToJson(false, false, nullptr)); EXPECT_JSON_EQ("{'enum':[true,false],'type':'boolean'}", - *param2.ToJson(true, nullptr)); + *param2.ToJson(true, false, nullptr)); param2.FromJson(CreateDictionaryValue("{'default':true}").get(), &prop, nullptr); - EXPECT_JSON_EQ("{'default':true}", *param2.ToJson(false, nullptr)); + EXPECT_JSON_EQ("{'default':true}", *param2.ToJson(false, false, nullptr)); } TEST(CommandSchema, BoolPropType_FromJson) { @@ -261,6 +263,7 @@ EXPECT_FALSE(prop.HasOverriddenAttributes()); EXPECT_FALSE(prop.IsBasedOnSchema()); EXPECT_EQ(nullptr, prop.GetDefaultValue()); + EXPECT_FALSE(prop.IsRequired()); } TEST(CommandSchema, DoublePropType_Types) { @@ -275,23 +278,23 @@ TEST(CommandSchema, DoublePropType_ToJson) { DoublePropType prop; - EXPECT_JSON_EQ("'number'", *prop.ToJson(false, nullptr)); - EXPECT_JSON_EQ("{'type':'number'}", *prop.ToJson(true, nullptr)); + EXPECT_JSON_EQ("'number'", *prop.ToJson(false, false, nullptr)); + EXPECT_JSON_EQ("{'type':'number'}", *prop.ToJson(true, false, nullptr)); DoublePropType param2; param2.FromJson(CreateDictionaryValue("{}").get(), &prop, nullptr); - EXPECT_JSON_EQ("{}", *param2.ToJson(false, nullptr)); + EXPECT_JSON_EQ("{}", *param2.ToJson(false, false, nullptr)); param2.FromJson(CreateDictionaryValue("{'minimum':3}").get(), &prop, nullptr); - EXPECT_JSON_EQ("{'minimum':3.0}", *param2.ToJson(false, nullptr)); + EXPECT_JSON_EQ("{'minimum':3.0}", *param2.ToJson(false, false, nullptr)); param2.FromJson(CreateDictionaryValue("{'maximum':-7}").get(), &prop, nullptr); - EXPECT_JSON_EQ("{'maximum':-7.0}", *param2.ToJson(false, nullptr)); + EXPECT_JSON_EQ("{'maximum':-7.0}", *param2.ToJson(false, false, nullptr)); param2.FromJson(CreateDictionaryValue("{'minimum':0,'maximum':5}").get(), &prop, nullptr); EXPECT_JSON_EQ("{'maximum':5.0,'minimum':0.0}", - *param2.ToJson(false, nullptr)); + *param2.ToJson(false, false, nullptr)); param2.FromJson(CreateDictionaryValue("{'default':12.3}").get(), &prop, nullptr); - EXPECT_JSON_EQ("{'default':12.3}", *param2.ToJson(false, nullptr)); + EXPECT_JSON_EQ("{'default':12.3}", *param2.ToJson(false, false, nullptr)); } TEST(CommandSchema, DoublePropType_FromJson) { @@ -378,6 +381,7 @@ EXPECT_FALSE(prop.HasOverriddenAttributes()); EXPECT_FALSE(prop.IsBasedOnSchema()); EXPECT_EQ(nullptr, prop.GetDefaultValue()); + EXPECT_FALSE(prop.IsRequired()); } TEST(CommandSchema, StringPropType_Types) { @@ -392,24 +396,24 @@ TEST(CommandSchema, StringPropType_ToJson) { StringPropType prop; - EXPECT_JSON_EQ("'string'", *prop.ToJson(false, nullptr)); - EXPECT_JSON_EQ("{'type':'string'}", *prop.ToJson(true, nullptr)); + EXPECT_JSON_EQ("'string'", *prop.ToJson(false, false, nullptr)); + EXPECT_JSON_EQ("{'type':'string'}", *prop.ToJson(true, false, nullptr)); StringPropType param2; param2.FromJson(CreateDictionaryValue("{}").get(), &prop, nullptr); - EXPECT_JSON_EQ("{}", *param2.ToJson(false, nullptr)); + EXPECT_JSON_EQ("{}", *param2.ToJson(false, false, nullptr)); param2.FromJson(CreateDictionaryValue("{'minLength':3}").get(), &prop, nullptr); - EXPECT_JSON_EQ("{'minLength':3}", *param2.ToJson(false, nullptr)); + EXPECT_JSON_EQ("{'minLength':3}", *param2.ToJson(false, false, nullptr)); param2.FromJson(CreateDictionaryValue("{'maxLength':7}").get(), &prop, nullptr); - EXPECT_JSON_EQ("{'maxLength':7}", *param2.ToJson(false, nullptr)); + EXPECT_JSON_EQ("{'maxLength':7}", *param2.ToJson(false, false, nullptr)); param2.FromJson(CreateDictionaryValue("{'minLength':0,'maxLength':5}").get(), &prop, nullptr); EXPECT_JSON_EQ("{'maxLength':5,'minLength':0}", - *param2.ToJson(false, nullptr)); + *param2.ToJson(false, false, nullptr)); param2.FromJson(CreateDictionaryValue("{'default':'abcd'}").get(), &prop, nullptr); - EXPECT_JSON_EQ("{'default':'abcd'}", *param2.ToJson(false, nullptr)); + EXPECT_JSON_EQ("{'default':'abcd'}", *param2.ToJson(false, false, nullptr)); } TEST(CommandSchema, StringPropType_FromJson) { @@ -501,6 +505,7 @@ EXPECT_TRUE(prop.HasOverriddenAttributes()); EXPECT_FALSE(prop.IsBasedOnSchema()); EXPECT_EQ(nullptr, prop.GetDefaultValue()); + EXPECT_FALSE(prop.IsRequired()); } TEST(CommandSchema, ObjectPropType_Types) { @@ -516,14 +521,14 @@ TEST(CommandSchema, ObjectPropType_ToJson) { ObjectPropType prop; EXPECT_JSON_EQ("{'additionalProperties':false,'properties':{}}", - *prop.ToJson(false, nullptr)); + *prop.ToJson(false, false, nullptr)); EXPECT_JSON_EQ( "{'additionalProperties':false,'properties':{},'type':'object'}", - *prop.ToJson(true, nullptr)); + *prop.ToJson(true, false, nullptr)); EXPECT_FALSE(prop.IsBasedOnSchema()); ObjectPropType prop2; prop2.FromJson(CreateDictionaryValue("{}").get(), &prop, nullptr); - EXPECT_JSON_EQ("{}", *prop2.ToJson(false, nullptr)); + EXPECT_JSON_EQ("{}", *prop2.ToJson(false, false, nullptr)); EXPECT_TRUE(prop2.IsBasedOnSchema()); auto schema = ObjectSchema::Create(); @@ -542,7 +547,7 @@ } } })"; - EXPECT_JSON_EQ(expected, *prop2.ToJson(false, nullptr)); + EXPECT_JSON_EQ(expected, *prop2.ToJson(false, false, nullptr)); expected = R"({ 'additionalProperties': false, @@ -558,7 +563,7 @@ }, 'type': 'object' })"; - EXPECT_JSON_EQ(expected, *prop2.ToJson(true, nullptr)); + EXPECT_JSON_EQ(expected, *prop2.ToJson(true, false, nullptr)); ObjectPropType prop3; ASSERT_TRUE( @@ -572,7 +577,7 @@ 'password': 'abracadabra' } })"; - EXPECT_JSON_EQ(expected, *prop3.ToJson(false, nullptr)); + EXPECT_JSON_EQ(expected, *prop3.ToJson(false, false, nullptr)); expected = R"({ 'additionalProperties': false, @@ -592,7 +597,7 @@ }, 'type': 'object' })"; - EXPECT_JSON_EQ(expected, *prop3.ToJson(true, nullptr)); + EXPECT_JSON_EQ(expected, *prop3.ToJson(true, false, nullptr)); ObjectPropType prop4; ASSERT_TRUE( @@ -615,7 +620,7 @@ } } })"; - EXPECT_JSON_EQ(expected, *prop4.ToJson(false, nullptr)); + EXPECT_JSON_EQ(expected, *prop4.ToJson(false, false, nullptr)); expected = R"({ 'additionalProperties': true, @@ -635,7 +640,7 @@ }, 'type': 'object' })"; - EXPECT_JSON_EQ(expected, *prop4.ToJson(true, nullptr)); + EXPECT_JSON_EQ(expected, *prop4.ToJson(true, false, nullptr)); } TEST(CommandSchema, ObjectPropType_FromJson) { @@ -669,8 +674,8 @@ ObjectPropType prop; prop.FromJson(CreateDictionaryValue( "{'properties':{'expires':'integer'," - "'password':{'maxLength':100,'minLength':6}}}") - .get(), + "'password':{'maxLength':100,'minLength':6}}," + "'required':['expires','password']}").get(), nullptr, nullptr); chromeos::ErrorPtr error; EXPECT_TRUE(prop.ValidateValue( @@ -763,6 +768,7 @@ EXPECT_TRUE(prop.HasOverriddenAttributes()); EXPECT_FALSE(prop.IsBasedOnSchema()); EXPECT_NE(nullptr, prop.GetItemTypePtr()); + EXPECT_FALSE(prop.IsRequired()); } TEST(CommandSchema, ArrayPropType_Types) { @@ -778,20 +784,20 @@ TEST(CommandSchema, ArrayPropType_ToJson) { ArrayPropType prop; prop.SetItemType(PropType::Create(ValueType::Int)); - EXPECT_JSON_EQ("{'items':'integer'}", *prop.ToJson(false, nullptr)); + EXPECT_JSON_EQ("{'items':'integer'}", *prop.ToJson(false, false, nullptr)); EXPECT_JSON_EQ("{'items':{'type':'integer'},'type':'array'}", - *prop.ToJson(true, nullptr)); + *prop.ToJson(true, false, nullptr)); EXPECT_FALSE(prop.IsBasedOnSchema()); ArrayPropType prop2; prop2.FromJson(CreateDictionaryValue("{}").get(), &prop, nullptr); - EXPECT_JSON_EQ("{}", *prop2.ToJson(false, nullptr)); + EXPECT_JSON_EQ("{}", *prop2.ToJson(false, false, nullptr)); EXPECT_TRUE(prop2.IsBasedOnSchema()); prop2.FromJson(CreateDictionaryValue("{'default':[1,2,3]}").get(), &prop, nullptr); - EXPECT_JSON_EQ("{'default':[1,2,3]}", *prop2.ToJson(false, nullptr)); + EXPECT_JSON_EQ("{'default':[1,2,3]}", *prop2.ToJson(false, false, nullptr)); EXPECT_JSON_EQ( "{'default':[1,2,3],'items':{'type':'integer'},'type':'array'}", - *prop2.ToJson(true, nullptr)); + *prop2.ToJson(true, false, nullptr)); } TEST(CommandSchema, ArrayPropType_FromJson) { @@ -1474,4 +1480,238 @@ error.reset(); } +TEST(CommandSchema, RequiredProperties_Integral) { + IntPropType prop; + + prop.MakeRequired(false); + EXPECT_JSON_EQ("{'type':'integer'}", *prop.ToJson(true, false, nullptr)); + EXPECT_JSON_EQ("{'isRequired':false,'type':'integer'}", + *prop.ToJson(true, true, nullptr)); + + prop.MakeRequired(true); + EXPECT_JSON_EQ("{'type':'integer'}", *prop.ToJson(true, false, nullptr)); + EXPECT_JSON_EQ("{'isRequired':true,'type':'integer'}", + *prop.ToJson(true, true, nullptr)); + + IntPropType prop2; + EXPECT_TRUE(prop2.FromJson(CreateDictionaryValue("{}").get(), &prop, + nullptr)); + EXPECT_TRUE(prop2.IsRequired()); + + EXPECT_TRUE(prop2.FromJson( + CreateDictionaryValue("{'isRequired': false}").get(), &prop, nullptr)); + EXPECT_FALSE(prop2.IsRequired()); + + EXPECT_JSON_EQ("{'type':'integer'}", *prop2.ToJson(true, false, nullptr)); + EXPECT_JSON_EQ("{'isRequired':false,'type':'integer'}", + *prop2.ToJson(true, true, nullptr)); +} + +TEST(CommandSchema, RequiredProperties_Object) { + ObjectPropType obj_type; + auto schema = ObjectSchema::Create(); + auto type = PropType::Create(ValueType::Int); + type->MakeRequired(true); + schema->AddProp("prop1", std::move(type)); + type = PropType::Create(ValueType::String); + type->MakeRequired(false); + schema->AddProp("prop2", std::move(type)); + type = PropType::Create(ValueType::Boolean); + type->MakeRequired(true); + schema->AddProp("prop3", std::move(type)); + type = PropType::Create(ValueType::Array); + type->GetArray()->SetItemType(PropType::Create(ValueType::String)); + schema->AddProp("prop4", std::move(type)); + auto expected1 = R"({ + 'prop1': 'integer', + 'prop2': 'string', + 'prop3': 'boolean', + 'prop4': {'items': 'string'} + })"; + EXPECT_JSON_EQ(expected1, *schema->ToJson(false, false, nullptr)); + auto expected2 = R"({ + 'prop1': {'type':'integer','isRequired': true}, + 'prop2': {'type':'string','isRequired': false}, + 'prop3': {'type':'boolean','isRequired': true}, + 'prop4': {'items': 'string'} + })"; + EXPECT_JSON_EQ(expected2, *schema->ToJson(false, true, nullptr)); + + obj_type.SetObjectSchema(std::move(schema)); + auto expected3 = R"({ + 'additionalProperties': false, + 'properties': { + 'prop1': 'integer', + 'prop2': 'string', + 'prop3': 'boolean', + 'prop4': {'items': 'string'} + }, + 'required': ['prop1','prop3'] + })"; + EXPECT_JSON_EQ(expected3, *obj_type.ToJson(false, false, nullptr)); + EXPECT_JSON_EQ(expected3, *obj_type.ToJson(false, true, nullptr)); +} + +TEST(CommandSchema, RequiredProperties_Schema_FromJson) { + ObjectSchema schema; + auto schema_str = R"({ + 'prop1': {'type':'integer','isRequired': true}, + 'prop2': {'type':'string','isRequired': false}, + 'prop3': 'boolean' + })"; + EXPECT_TRUE(schema.FromJson(CreateDictionaryValue(schema_str).get(), nullptr, + nullptr)); + EXPECT_TRUE(schema.GetProp("prop1")->IsRequired()); + EXPECT_FALSE(schema.GetProp("prop2")->IsRequired()); + EXPECT_FALSE(schema.GetProp("prop3")->IsRequired()); + EXPECT_JSON_EQ(schema_str, *schema.ToJson(false, true, nullptr)); +} + +TEST(CommandSchema, RequiredProperties_Schema_FromJson_Inherit) { + ObjectSchema base_schema; + auto base_schema_str = R"({ + 'prop1': {'type':'integer','isRequired': true}, + 'prop2': {'type':'integer','isRequired': false}, + 'prop3': {'type':'integer','isRequired': true}, + 'prop4': {'type':'integer','isRequired': false} + })"; + EXPECT_TRUE(base_schema.FromJson(CreateDictionaryValue(base_schema_str).get(), + nullptr, nullptr)); + ObjectSchema schema; + auto schema_str = R"({ + 'prop1': {'isRequired': false}, + 'prop2': {'isRequired': true}, + 'prop3': {}, + 'prop4': 'integer' + })"; + EXPECT_TRUE(schema.FromJson(CreateDictionaryValue(schema_str).get(), + &base_schema, nullptr)); + EXPECT_FALSE(schema.GetProp("prop1")->IsRequired()); + EXPECT_TRUE(schema.GetProp("prop2")->IsRequired()); + EXPECT_TRUE(schema.GetProp("prop3")->IsRequired()); + EXPECT_FALSE(schema.GetProp("prop4")->IsRequired()); + auto expected = R"({ + 'prop1': {'type':'integer','isRequired': false}, + 'prop2': {'type':'integer','isRequired': true}, + 'prop3': {}, + 'prop4': {} + })"; + EXPECT_JSON_EQ(expected, *schema.ToJson(false, true, nullptr)); +} + +TEST(CommandSchema, RequiredProperties_ObjectPropType_FromJson) { + ObjectPropType obj_type; + auto type_str = R"({ + 'properties': { + 'prop1': 'integer', + 'prop2': 'string', + 'prop3': {'type':'boolean','isRequired':true}, + 'prop4': {'items': 'string','isRequired':false}, + 'prop5': {'type':'number','isRequired':true} + }, + 'required': ['prop1','prop3','prop4','prop5'] + })"; + EXPECT_TRUE(obj_type.FromJson(CreateDictionaryValue(type_str).get(), nullptr, + nullptr)); + EXPECT_TRUE(obj_type.GetObjectSchemaPtr()->GetProp("prop1")->IsRequired()); + EXPECT_FALSE(obj_type.GetObjectSchemaPtr()->GetProp("prop2")->IsRequired()); + EXPECT_TRUE(obj_type.GetObjectSchemaPtr()->GetProp("prop3")->IsRequired()); + // 'required' takes precedence over 'isRequired'. + EXPECT_TRUE(obj_type.GetObjectSchemaPtr()->GetProp("prop4")->IsRequired()); + EXPECT_TRUE(obj_type.GetObjectSchemaPtr()->GetProp("prop5")->IsRequired()); +} + +TEST(CommandSchema, RequiredProperties_Failures) { + ObjectPropType obj_type; + chromeos::ErrorPtr error; + + auto type_str = R"({ + 'properties': { + 'prop1': 'integer', + 'prop2': 'string' + }, + 'required': ['prop1','prop3','prop4'] + })"; + EXPECT_FALSE(obj_type.FromJson(CreateDictionaryValue(type_str).get(), nullptr, + &error)); + EXPECT_EQ(errors::commands::kUnknownProperty, error->GetCode()); + EXPECT_EQ("Unknown property 'prop3'", error->GetMessage()); + error.reset(); + + type_str = R"({ + 'properties': { + 'prop1': 'integer', + 'prop2': 'string' + }, + 'required': 'prop1' + })"; + EXPECT_FALSE(obj_type.FromJson(CreateDictionaryValue(type_str).get(), nullptr, + &error)); + EXPECT_EQ(errors::commands::kInvalidObjectSchema, error->GetCode()); + EXPECT_EQ("Property 'required' must be an array", error->GetMessage()); + error.reset(); +} + +TEST(CommandSchema, ObjectSchema_UseRequired) { + ObjectPropType prop; + auto schema_str = R"({ + 'properties':{ + 'param1':'integer', + 'param2':'integer', + 'param3':{'default':3}, + 'param4':{'default':4} + }, + 'required':['param1','param3'] + })"; + ASSERT_TRUE(prop.FromJson(CreateDictionaryValue(schema_str).get(), nullptr, + nullptr)); + + auto value = prop.CreateValue(); + auto val_json = R"({ + 'param1':10, + 'param2':20, + 'param3':30, + 'param4':40 + })"; + ASSERT_TRUE(value->FromJson(CreateDictionaryValue(val_json).get(), nullptr)); + native_types::Object obj = value->GetObject()->GetValue(); + EXPECT_EQ(10, obj["param1"]->GetInt()->GetValue()); + EXPECT_EQ(20, obj["param2"]->GetInt()->GetValue()); + EXPECT_EQ(30, obj["param3"]->GetInt()->GetValue()); + EXPECT_EQ(40, obj["param4"]->GetInt()->GetValue()); + + value = prop.CreateValue(); + val_json = "{'param1':100}"; + ASSERT_TRUE(value->FromJson(CreateDictionaryValue(val_json).get(), nullptr)); + obj = value->GetObject()->GetValue(); + EXPECT_EQ(3, obj.size()); + + EXPECT_EQ(100, obj["param1"]->GetInt()->GetValue()); + EXPECT_EQ(obj.end(), obj.find("param2")); + EXPECT_EQ(3, obj["param3"]->GetInt()->GetValue()); + EXPECT_EQ(4, obj["param4"]->GetInt()->GetValue()); +} + +TEST(CommandSchema, ObjectSchema_UseRequired_Failure) { + ObjectPropType prop; + auto schema_str = R"({ + 'properties':{ + 'param1':'integer', + 'param2':'integer', + 'param3':{'default':3}, + 'param4':{'default':4} + }, + 'required':['param1','param3'] + })"; + ASSERT_TRUE(prop.FromJson(CreateDictionaryValue(schema_str).get(), nullptr, + nullptr)); + + auto value = prop.CreateValue(); + auto val_json = "{'param2':20}"; + chromeos::ErrorPtr error; + ASSERT_FALSE(value->FromJson(CreateDictionaryValue(val_json).get(), &error)); + EXPECT_EQ(errors::commands::kPropertyMissing, error->GetCode()); + EXPECT_EQ("Required parameter missing: param1", error->GetMessage()); +} + } // namespace buffet
diff --git a/libweave/src/commands/prop_types.cc b/libweave/src/commands/prop_types.cc index bfd1b50..eb4fc6e 100644 --- a/libweave/src/commands/prop_types.cc +++ b/libweave/src/commands/prop_types.cc
@@ -42,8 +42,27 @@ return false; } +bool PropType::IsRequired() const { + return required_.value; +} + +void PropType::MakeRequired(bool required) { + required_.value = required; + required_.is_inherited = false; +} + std::unique_ptr<base::Value> PropType::ToJson(bool full_schema, + bool in_command_def, chromeos::ErrorPtr* error) const { + // Determine if we need to output "isRequired" attribute. + const bool include_required = in_command_def && !required_.is_inherited; + + // If we must include "isRequired" attribute, then treat this as "full schema" + // request because there could be cases where we have just this attribute and + // won't be able to infer the type from the constraints only. + if (include_required) + full_schema = true; + if (!full_schema && !HasOverriddenAttributes()) { if (based_on_schema_) return std::unique_ptr<base::Value>(new base::DictionaryValue); @@ -90,6 +109,8 @@ dict->Set(commands::attributes::kDefault, defval.release()); } + if (include_required) + dict->SetBoolean(commands::attributes::kIsRequired, required_.value); return std::unique_ptr<base::Value>(dict.release()); } @@ -102,6 +123,7 @@ cloned->default_.is_inherited = default_.is_inherited; if (default_.value) cloned->default_.value = default_.value->Clone(); + cloned->required_ = required_; return cloned; } @@ -125,6 +147,7 @@ commands::attributes::kType, commands::attributes::kDisplayName, commands::attributes::kDefault, + commands::attributes::kIsRequired, }; if (!ObjectSchemaFromJson(value, base_schema, &processed_keys, error)) return false; @@ -150,6 +173,18 @@ iter.Advance(); } + // Read the "isRequired" attribute, if specified. + bool required = false; + if (value->GetBoolean(commands::attributes::kIsRequired, &required)) { + required_.value = required; + required_.is_inherited = false; + } else if (base_schema) { + // If we have the base schema, inherit the type's required value from it. + if (base_schema->required_.value) + required_.value = base_schema->required_.value; + required_.is_inherited = true; + } + // Read the default value, if specified. // We need to do this last since the current type definition must be complete, // so we can parse and validate the value of the default. @@ -462,13 +497,16 @@ std::unique_ptr<base::Value> ObjectPropType::ToJson( bool full_schema, + bool in_command_def, chromeos::ErrorPtr* error) const { - std::unique_ptr<base::Value> value = PropType::ToJson(full_schema, error); + std::unique_ptr<base::Value> value = + PropType::ToJson(full_schema, in_command_def, error); if (value) { base::DictionaryValue* dict = nullptr; CHECK(value->GetAsDictionary(&dict)) << "Expecting a JSON object"; if (!object_schema_.is_inherited || full_schema) { - auto object_schema = object_schema_.value->ToJson(full_schema, error); + auto object_schema = + object_schema_.value->ToJson(full_schema, false, error); if (!object_schema) { value.reset(); return value; @@ -478,6 +516,14 @@ dict->SetBooleanWithoutPathExpansion( commands::attributes::kObject_AdditionalProperties, object_schema_.value->GetExtraPropertiesAllowed()); + std::unique_ptr<base::ListValue> required{new base::ListValue}; + for (const auto& pair : object_schema_.value->GetProps()) { + if (pair.second->IsRequired()) + required->AppendString(pair.first); + } + if (required->GetSize() > 0) { + dict->Set(commands::attributes::kObject_Required, required.release()); + } } } return value; @@ -498,35 +544,20 @@ base_object_schema = base_schema->GetObject()->GetObjectSchemaPtr(); const base::DictionaryValue* props = nullptr; + std::unique_ptr<ObjectSchema> object_schema; + bool inherited = false; if (value->GetDictionaryWithoutPathExpansion(kObject_Properties, &props)) { processed_keys->insert(kObject_Properties); - std::unique_ptr<ObjectSchema> object_schema{new ObjectSchema}; + object_schema.reset(new ObjectSchema); if (!object_schema->FromJson(props, base_object_schema, error)) { chromeos::Error::AddTo(error, FROM_HERE, errors::commands::kDomain, errors::commands::kInvalidObjectSchema, "Error parsing object property schema"); return false; } - bool extra_properties_allowed = false; - if (value->GetBooleanWithoutPathExpansion(kObject_AdditionalProperties, - &extra_properties_allowed)) { - processed_keys->insert(kObject_AdditionalProperties); - object_schema->SetExtraPropertiesAllowed(extra_properties_allowed); - } - object_schema_.value = std::move(object_schema); - object_schema_.is_inherited = false; } else if (base_object_schema) { - auto cloned_object_schema = base_object_schema->Clone(); - bool extra_properties_allowed = false; - if (value->GetBooleanWithoutPathExpansion(kObject_AdditionalProperties, - &extra_properties_allowed)) { - processed_keys->insert(kObject_AdditionalProperties); - cloned_object_schema->SetExtraPropertiesAllowed(extra_properties_allowed); - object_schema_.is_inherited = false; - } else { - object_schema_.is_inherited = true; - } - object_schema_.value = std::move(cloned_object_schema); + object_schema = base_object_schema->Clone(); + inherited = true; } else { chromeos::Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain, errors::commands::kInvalidObjectSchema, @@ -535,6 +566,43 @@ kObject_Properties); return false; } + bool extra_properties_allowed = false; + if (value->GetBooleanWithoutPathExpansion(kObject_AdditionalProperties, + &extra_properties_allowed)) { + processed_keys->insert(kObject_AdditionalProperties); + object_schema->SetExtraPropertiesAllowed(extra_properties_allowed); + inherited = false; + } + const base::Value* required = nullptr; + if (value->Get(commands::attributes::kObject_Required, &required)) { + processed_keys->insert(commands::attributes::kObject_Required); + const base::ListValue* required_list = nullptr; + if (!required->GetAsList(&required_list)) { + chromeos::Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain, + errors::commands::kInvalidObjectSchema, + "Property '%s' must be an array", + commands::attributes::kObject_Required); + return false; + } + for (const base::Value* value : *required_list) { + std::string name; + if (!value->GetAsString(&name)) { + std::string json_value; + CHECK(base::JSONWriter::Write(*value, &json_value)); + chromeos::Error::AddToPrintf( + error, FROM_HERE, errors::commands::kDomain, + errors::commands::kInvalidObjectSchema, + "Property '%s' contains invalid element (%s). String expected", + commands::attributes::kObject_Required, json_value.c_str()); + return false; + } + if (!object_schema->MarkPropRequired(name, error)) + return false; + inherited = false; + } + } + object_schema_.value = std::move(object_schema); + object_schema_.is_inherited = inherited; return true; } @@ -594,20 +662,20 @@ std::unique_ptr<base::Value> ArrayPropType::ToJson( bool full_schema, + bool in_command_def, chromeos::ErrorPtr* error) const { - std::unique_ptr<base::Value> value = PropType::ToJson(full_schema, error); - if (value) { + std::unique_ptr<base::Value> value = + PropType::ToJson(full_schema, in_command_def, error); + if (value && (!item_type_.is_inherited || full_schema)) { base::DictionaryValue* dict = nullptr; CHECK(value->GetAsDictionary(&dict)) << "Expecting a JSON object"; - if (!item_type_.is_inherited || full_schema) { - auto type = item_type_.value->ToJson(full_schema, error); + auto type = item_type_.value->ToJson(full_schema, false, error); if (!type) { value.reset(); return value; } dict->SetWithoutPathExpansion(commands::attributes::kItems, type.release()); - } } return value; }
diff --git a/libweave/src/commands/prop_types.h b/libweave/src/commands/prop_types.h index 156b556..9acab29 100644 --- a/libweave/src/commands/prop_types.h +++ b/libweave/src/commands/prop_types.h
@@ -55,6 +55,12 @@ const PropValue* GetDefaultValue() const { return default_.value.get(); } // Gets the constraints specified for the parameter, if any. const ConstraintMap& GetConstraints() const { return constraints_; } + // Returns true if this value is required. Properties are marked as required + // by using "isRequired" attribute or listed in "required" array. + bool IsRequired() const; + // Sets the required attribute to the value of |required| and marks it as + // overridden (not-inherited). + void MakeRequired(bool required); // Checks if any of the type attributes were overridden from the base // schema definition. If this type does not inherit from a base schema, // this method returns true. @@ -109,7 +115,12 @@ // saved. // If it fails, returns "nullptr" and fills in the |error| with additional // error information. + // |in_command_def| is set to true if the property type describes a + // GCD command parameter, otherwise it is for an object property. + // Command definitions handle required parameters differently (using + // "isRequired" property as opposed to "required" list for object properties). virtual std::unique_ptr<base::Value> ToJson(bool full_schema, + bool in_command_def, chromeos::ErrorPtr* error) const; // Parses an JSON parameter type definition. Optional |base_schema| may // specify the base schema type definition this type should be based upon. @@ -190,6 +201,11 @@ // Otherwise the parameter is treated as required and, if it is omitted, // this is treated as an error. InheritableAttribute<std::unique_ptr<PropValue>> default_; + // Specifies whether the parameter/property is required and must be specified + // (either directly, or by the default value being provided in the schema). + // Non-required parameters can be omitted completely and their values will not + // be present in the object instance. + InheritableAttribute<bool> required_; }; // Base class for all the derived concrete implementations of property @@ -346,6 +362,7 @@ std::unique_ptr<PropType> Clone() const override; std::unique_ptr<base::Value> ToJson(bool full_schema, + bool in_command_def, chromeos::ErrorPtr* error) const override; bool ObjectSchemaFromJson(const base::DictionaryValue* value, const PropType* base_schema, @@ -385,6 +402,7 @@ std::unique_ptr<PropType> Clone() const override; std::unique_ptr<base::Value> ToJson(bool full_schema, + bool in_command_def, chromeos::ErrorPtr* error) const override; bool ObjectSchemaFromJson(const base::DictionaryValue* value,
diff --git a/libweave/src/commands/schema_constants.cc b/libweave/src/commands/schema_constants.cc index 733c66d..2cb1dfb 100644 --- a/libweave/src/commands/schema_constants.cc +++ b/libweave/src/commands/schema_constants.cc
@@ -35,6 +35,7 @@ const char kDisplayName[] = "displayName"; const char kDefault[] = "default"; const char kItems[] = "items"; +const char kIsRequired[] = "isRequired"; const char kNumeric_Min[] = "minimum"; const char kNumeric_Max[] = "maximum"; @@ -47,6 +48,7 @@ const char kObject_Properties[] = "properties"; const char kObject_AdditionalProperties[] = "additionalProperties"; +const char kObject_Required[] = "required"; const char kCommand_Id[] = "id"; const char kCommand_Name[] = "name";
diff --git a/libweave/src/commands/schema_constants.h b/libweave/src/commands/schema_constants.h index e00bae5..379c729 100644 --- a/libweave/src/commands/schema_constants.h +++ b/libweave/src/commands/schema_constants.h
@@ -38,6 +38,7 @@ extern const char kDisplayName[]; extern const char kDefault[]; extern const char kItems[]; +extern const char kIsRequired[]; extern const char kNumeric_Min[]; extern const char kNumeric_Max[]; @@ -50,6 +51,7 @@ extern const char kObject_Properties[]; extern const char kObject_AdditionalProperties[]; +extern const char kObject_Required[]; extern const char kCommand_Id[]; extern const char kCommand_Name[];
diff --git a/libweave/src/commands/schema_utils.cc b/libweave/src/commands/schema_utils.cc index c0067ab..1575fdc 100644 --- a/libweave/src/commands/schema_utils.cc +++ b/libweave/src/commands/schema_utils.cc
@@ -17,12 +17,13 @@ namespace buffet { namespace { // Helper function to report "type mismatch" errors when parsing JSON. -void ReportJsonTypeMismatch(const base::Value* value_in, +void ReportJsonTypeMismatch(const tracked_objects::Location& location, + const base::Value* value_in, const std::string& expected_type, chromeos::ErrorPtr* error) { std::string value_as_string; base::JSONWriter::Write(*value_in, &value_as_string); - chromeos::Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain, + chromeos::Error::AddToPrintf(error, location, errors::commands::kDomain, errors::commands::kTypeMismatch, "Unable to convert value %s into %s", value_as_string.c_str(), expected_type.c_str()); @@ -32,16 +33,20 @@ // data from the value_out parameter passed to particular overload of // TypedValueFromJson() function. Always returns false. template <typename T> -bool ReportUnexpectedJson(const base::Value* value_in, +bool ReportUnexpectedJson(const tracked_objects::Location& location, + const base::Value* value_in, T*, chromeos::ErrorPtr* error) { - ReportJsonTypeMismatch( - value_in, PropType::GetTypeStringFromType(GetValueType<T>()), error); + ReportJsonTypeMismatch(location, value_in, + PropType::GetTypeStringFromType(GetValueType<T>()), + error); return false; } -bool ErrorMissingProperty(chromeos::ErrorPtr* error, const char* param_name) { - chromeos::Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain, +bool ErrorMissingProperty(chromeos::ErrorPtr* error, + const tracked_objects::Location& location, + const char* param_name) { + chromeos::Error::AddToPrintf(error, location, errors::commands::kDomain, errors::commands::kPropertyMissing, "Required parameter missing: %s", param_name); return false; @@ -98,7 +103,7 @@ bool* value_out, chromeos::ErrorPtr* error) { return value_in->GetAsBoolean(value_out) || - ReportUnexpectedJson(value_in, value_out, error); + ReportUnexpectedJson(FROM_HERE, value_in, value_out, error); } bool TypedValueFromJson(const base::Value* value_in, @@ -106,7 +111,7 @@ int* value_out, chromeos::ErrorPtr* error) { return value_in->GetAsInteger(value_out) || - ReportUnexpectedJson(value_in, value_out, error); + ReportUnexpectedJson(FROM_HERE, value_in, value_out, error); } bool TypedValueFromJson(const base::Value* value_in, @@ -114,7 +119,7 @@ double* value_out, chromeos::ErrorPtr* error) { return value_in->GetAsDouble(value_out) || - ReportUnexpectedJson(value_in, value_out, error); + ReportUnexpectedJson(FROM_HERE, value_in, value_out, error); } bool TypedValueFromJson(const base::Value* value_in, @@ -122,7 +127,7 @@ std::string* value_out, chromeos::ErrorPtr* error) { return value_in->GetAsString(value_out) || - ReportUnexpectedJson(value_in, value_out, error); + ReportUnexpectedJson(FROM_HERE, value_in, value_out, error); } bool TypedValueFromJson(const base::Value* value_in, @@ -131,7 +136,7 @@ chromeos::ErrorPtr* error) { const base::DictionaryValue* dict = nullptr; if (!value_in->GetAsDictionary(&dict)) - return ReportUnexpectedJson(value_in, value_out, error); + return ReportUnexpectedJson(FROM_HERE, value_in, value_out, error); CHECK(type) << "Object definition must be provided"; CHECK(ValueType::Object == type->GetType()) << "Type must be Object"; @@ -155,12 +160,13 @@ return false; } value_out->emplace_hint(value_out->end(), pair.first, std::move(value)); + keys_processed.insert(pair.first); } else if (def_value) { value_out->emplace_hint(value_out->end(), pair.first, def_value->Clone()); - } else { - return ErrorMissingProperty(error, pair.first.c_str()); + keys_processed.insert(pair.first); + } else if (pair.second->IsRequired()) { + return ErrorMissingProperty(error, FROM_HERE, pair.first.c_str()); } - keys_processed.insert(pair.first); } // Just for sanity, make sure that we processed all the necessary properties @@ -200,7 +206,7 @@ chromeos::ErrorPtr* error) { const base::ListValue* list = nullptr; if (!value_in->GetAsList(&list)) - return ReportUnexpectedJson(value_in, value_out, error); + return ReportUnexpectedJson(FROM_HERE, value_in, value_out, error); CHECK(type) << "Array type definition must be provided"; CHECK(ValueType::Array == type->GetType()) << "Type must be Array"; @@ -355,7 +361,7 @@ } else if (def_value) { obj->emplace_hint(obj->end(), pair.first, def_value->Clone()); } else { - ErrorMissingProperty(error, pair.first.c_str()); + ErrorMissingProperty(error, FROM_HERE, pair.first.c_str()); return false; } keys_processed.insert(pair.first);
diff --git a/libweave/src/states/state_package_unittest.cc b/libweave/src/states/state_package_unittest.cc index 0b860cd..c6372a9 100644 --- a/libweave/src/states/state_package_unittest.cc +++ b/libweave/src/states/state_package_unittest.cc
@@ -109,7 +109,7 @@ 'type': 'boolean' } })"; - EXPECT_JSON_EQ(expected, *GetTypes(package).ToJson(true, nullptr)); + EXPECT_JSON_EQ(expected, *GetTypes(package).ToJson(true, false, nullptr)); expected = R"({ 'color': '', @@ -171,7 +171,7 @@ 'type': 'boolean' } })"; - EXPECT_JSON_EQ(expected, *GetTypes(*package_).ToJson(true, nullptr)); + EXPECT_JSON_EQ(expected, *GetTypes(*package_).ToJson(true, false, nullptr)); expected = R"({ 'brightness': '',