buffet: Change OneOf constraint to use generic PropValue list Now ContraintOneOf contains a vector<PropValue> as opposed to vector<T>. This will enable support for array types, because the alternative would be to add explicit specializations for combinations of basic types (int, bool, double, string,... ) and a vector. BUG=brillo:107 TEST=`FEATURES=test emerge-link buffet` Change-Id: Ia6bd93db23517c463ba6d915617d9571499a8491 Reviewed-on: https://chromium-review.googlesource.com/261564 Reviewed-by: Vitaly Buka <vitalybuka@chromium.org> Commit-Queue: Alex Vakulenko <avakulenko@chromium.org> Tested-by: Alex Vakulenko <avakulenko@chromium.org> Trybot-Ready: Alex Vakulenko <avakulenko@chromium.org>
diff --git a/buffet/commands/object_schema_unittest.cc b/buffet/commands/object_schema_unittest.cc index 5e913f8..f257a67 100644 --- a/buffet/commands/object_schema_unittest.cc +++ b/buffet/commands/object_schema_unittest.cc
@@ -23,6 +23,25 @@ using buffet::unittests::CreateDictionaryValue; using buffet::unittests::ValueToString; +namespace { + +template<typename T> +std::vector<T> GetOneOfValues(const buffet::PropType* prop_type) { + std::vector<T> values; + auto one_of = static_cast<const buffet::ConstraintOneOf*>( + prop_type->GetConstraint(buffet::ConstraintType::OneOf)); + if (!one_of) + return values; + + values.reserve(one_of->set_.value.size()); + for (const auto& prop_value : one_of->set_.value) { + values.push_back(prop_value->GetValueAsAny().Get<T>()); + } + return values; +} + +} // anonymous namespace + TEST(CommandSchema, IntPropType_Empty) { buffet::IntPropType prop; EXPECT_TRUE(prop.GetConstraints().empty()); @@ -192,7 +211,7 @@ param2.FromJson(CreateDictionaryValue("{}").get(), &prop, nullptr); EXPECT_FALSE(param2.HasOverriddenAttributes()); EXPECT_TRUE(param2.IsBasedOnSchema()); - EXPECT_EQ(std::vector<bool>{true}, prop.GetOneOfValues()); + EXPECT_EQ(std::vector<bool>{true}, GetOneOfValues<bool>(&prop)); buffet::BooleanPropType prop_base; buffet::BooleanPropType param3; @@ -814,34 +833,25 @@ EXPECT_EQ("number", schema.GetProp("param9")->GetTypeAsString()); EXPECT_EQ("integer", schema.GetProp("param10")->GetTypeAsString()); - EXPECT_EQ(4, schema.GetProp("param1")->GetInt()->GetOneOfValues().size()); - EXPECT_EQ(3, schema.GetProp("param2")->GetDouble()->GetOneOfValues().size()); - EXPECT_EQ(2, schema.GetProp("param3")->GetString()->GetOneOfValues().size()); - EXPECT_EQ(3, schema.GetProp("param4")->GetInt()->GetOneOfValues().size()); - EXPECT_EQ(3, schema.GetProp("param5")->GetDouble()->GetOneOfValues().size()); - EXPECT_EQ(2, schema.GetProp("param6")->GetString()->GetOneOfValues().size()); - EXPECT_EQ(3, schema.GetProp("param7")->GetInt()->GetOneOfValues().size()); - EXPECT_EQ(3, schema.GetProp("param8")->GetDouble()->GetOneOfValues().size()); - EXPECT_EQ(0, schema.GetProp("param9")->GetDouble()->GetOneOfValues().size()); - EXPECT_EQ(0, schema.GetProp("param10")->GetInt()->GetOneOfValues().size()); + EXPECT_EQ((std::vector<int>{0, 1, 2, 3}), + GetOneOfValues<int>(schema.GetProp("param1"))); + EXPECT_EQ((std::vector<double>{0.0, 1.1, 2.2}), + GetOneOfValues<double>(schema.GetProp("param2"))); + EXPECT_EQ((std::vector<std::string>{"id1", "id2"}), + GetOneOfValues<std::string>(schema.GetProp("param3"))); - EXPECT_EQ(std::vector<int>({0, 1, 2, 3}), - schema.GetProp("param1")->GetInt()->GetOneOfValues()); - EXPECT_EQ(std::vector<double>({0.0, 1.1, 2.2}), - schema.GetProp("param2")->GetDouble()->GetOneOfValues()); - EXPECT_EQ(std::vector<std::string>({"id1", "id2"}), - schema.GetProp("param3")->GetString()->GetOneOfValues()); - - EXPECT_EQ(std::vector<int>({1, 2, 3}), - schema.GetProp("param4")->GetInt()->GetOneOfValues()); - EXPECT_EQ(std::vector<double>({-1.1, 2.2, 3.0}), - schema.GetProp("param5")->GetDouble()->GetOneOfValues()); - EXPECT_EQ(std::vector<std::string>({"id0", "id1"}), - schema.GetProp("param6")->GetString()->GetOneOfValues()); - EXPECT_EQ(std::vector<int>({1, 2, 3}), - schema.GetProp("param7")->GetInt()->GetOneOfValues()); - EXPECT_EQ(std::vector<double>({1.0, 2.0, 3.0}), - schema.GetProp("param8")->GetDouble()->GetOneOfValues()); + EXPECT_EQ((std::vector<int>{1, 2, 3}), + GetOneOfValues<int>(schema.GetProp("param4"))); + EXPECT_EQ((std::vector<double>{-1.1, 2.2, 3.0}), + GetOneOfValues<double>(schema.GetProp("param5"))); + EXPECT_EQ((std::vector<std::string>{"id0", "id1"}), + GetOneOfValues<std::string>(schema.GetProp("param6"))); + EXPECT_EQ((std::vector<int>{1, 2, 3}), + GetOneOfValues<int>(schema.GetProp("param7"))); + EXPECT_EQ((std::vector<double>{1.0, 2.0, 3.0}), + GetOneOfValues<double>(schema.GetProp("param8"))); + EXPECT_TRUE(GetOneOfValues<double>(schema.GetProp("param9")).empty()); + EXPECT_TRUE(GetOneOfValues<int>(schema.GetProp("param10")).empty()); } TEST(CommandSchema, ObjectSchema_FromJson_Inheritance) { @@ -939,23 +949,23 @@ EXPECT_EQ(3, schema.GetProp("param12")->GetString()->GetMinLength()); EXPECT_EQ(8, schema.GetProp("param12")->GetString()->GetMaxLength()); EXPECT_EQ("integer", schema.GetProp("param13")->GetTypeAsString()); - EXPECT_EQ(std::vector<int>({1, 2, 3}), - schema.GetProp("param13")->GetInt()->GetOneOfValues()); + EXPECT_EQ((std::vector<int>{1, 2, 3}), + GetOneOfValues<int>(schema.GetProp("param13"))); EXPECT_EQ("integer", schema.GetProp("param14")->GetTypeAsString()); - EXPECT_EQ(std::vector<int>({1, 2, 3, 4}), - schema.GetProp("param14")->GetInt()->GetOneOfValues()); + EXPECT_EQ((std::vector<int>{1, 2, 3, 4}), + GetOneOfValues<int>(schema.GetProp("param14"))); EXPECT_EQ("number", schema.GetProp("param15")->GetTypeAsString()); - EXPECT_EQ(std::vector<double>({1.1, 2.2, 3.3}), - schema.GetProp("param15")->GetDouble()->GetOneOfValues()); + EXPECT_EQ((std::vector<double>{1.1, 2.2, 3.3}), + GetOneOfValues<double>(schema.GetProp("param15"))); EXPECT_EQ("number", schema.GetProp("param16")->GetTypeAsString()); - EXPECT_EQ(std::vector<double>({1.1, 2.2, 3.3, 4.4}), - schema.GetProp("param16")->GetDouble()->GetOneOfValues()); + EXPECT_EQ((std::vector<double>{1.1, 2.2, 3.3, 4.4}), + GetOneOfValues<double>(schema.GetProp("param16"))); EXPECT_EQ("string", schema.GetProp("param17")->GetTypeAsString()); - EXPECT_EQ(std::vector<std::string>({"id1", "id2"}), - schema.GetProp("param17")->GetString()->GetOneOfValues()); + EXPECT_EQ((std::vector<std::string>{"id1", "id2"}), + GetOneOfValues<std::string>(schema.GetProp("param17"))); EXPECT_EQ("string", schema.GetProp("param18")->GetTypeAsString()); - EXPECT_EQ(std::vector<std::string>({"id1", "id3"}), - schema.GetProp("param18")->GetString()->GetOneOfValues()); + EXPECT_EQ((std::vector<std::string>{"id1", "id3"}), + GetOneOfValues<std::string>(schema.GetProp("param18"))); EXPECT_EQ("integer", schema.GetProp("param19")->GetTypeAsString()); EXPECT_EQ(1, schema.GetProp("param19")->GetInt()->GetMinValue()); EXPECT_EQ(5, schema.GetProp("param19")->GetInt()->GetMaxValue());
diff --git a/buffet/commands/prop_constraints.cc b/buffet/commands/prop_constraints.cc index 45448bc..085fe26 100644 --- a/buffet/commands/prop_constraints.cc +++ b/buffet/commands/prop_constraints.cc
@@ -4,11 +4,27 @@ #include "buffet/commands/prop_constraints.h" +#include <base/json/json_writer.h> + #include "buffet/commands/prop_values.h" #include "buffet/commands/schema_constants.h" namespace buffet { +namespace { + +// Helper function to convert a property value to string, which is used for +// error reporting. +std::string PropValueToString(const PropValue& value) { + std::string result; + auto json = value.ToJson(nullptr); + if (json) + base::JSONWriter::Write(json.get(), &result); + return result; +} + +} // anonymous namespace + // Constraint ---------------------------------------------------------------- Constraint::~Constraint() {} @@ -143,4 +159,59 @@ new ConstraintStringLengthMax{limit_.value}}; } +// ConstraintOneOf -------------------------------------------------- +ConstraintOneOf::ConstraintOneOf(InheritableAttribute<ChoiceList> set) + : set_(std::move(set)) {} +ConstraintOneOf::ConstraintOneOf(ChoiceList set) + : set_(std::move(set)) {} + +bool ConstraintOneOf::Validate(const PropValue& value, + chromeos::ErrorPtr* error) const { + for (const auto& item : set_.value) { + if (value.IsEqual(item.get())) + return true; + } + std::vector<std::string> choice_list; + choice_list.reserve(set_.value.size()); + for (const auto& item : set_.value) { + choice_list.push_back(PropValueToString(*item)); + } + return ReportErrorNotOneOf(error, PropValueToString(value), choice_list); +} + +std::unique_ptr<Constraint> ConstraintOneOf::Clone() const { + InheritableAttribute<ChoiceList> attr; + attr.is_inherited = set_.is_inherited; + attr.value.reserve(set_.value.size()); + for (const auto& prop_value : set_.value) { + attr.value.push_back(prop_value->Clone()); + } + return std::unique_ptr<Constraint>{new ConstraintOneOf{std::move(attr)}}; +} + +std::unique_ptr<Constraint> ConstraintOneOf::CloneAsInherited() const { + ChoiceList cloned; + cloned.reserve(set_.value.size()); + for (const auto& prop_value : set_.value) { + cloned.push_back(prop_value->Clone()); + } + return std::unique_ptr<Constraint>{new ConstraintOneOf{std::move(cloned)}}; +} + +std::unique_ptr<base::Value> ConstraintOneOf::ToJson( + chromeos::ErrorPtr* error) const { + std::unique_ptr<base::ListValue> list(new base::ListValue); + for (const auto& prop_value : set_.value) { + auto json = prop_value->ToJson(error); + if (!json) + return std::unique_ptr<base::Value>(); + list->Append(json.release()); + } + return std::move(list); +} + +const char* ConstraintOneOf::GetDictKey() const { + return commands::attributes::kOneOf_Enum; +} + } // namespace buffet
diff --git a/buffet/commands/prop_constraints.h b/buffet/commands/prop_constraints.h index 79cc72a..e5efdd2 100644 --- a/buffet/commands/prop_constraints.h +++ b/buffet/commands/prop_constraints.h
@@ -295,13 +295,12 @@ }; // Implementation of OneOf constraint for different data types. -template<typename T> class ConstraintOneOf : public Constraint { public: - explicit ConstraintOneOf(const InheritableAttribute<std::vector<T>>& set) - : set_(set) {} - explicit ConstraintOneOf(const std::vector<T>& set) - : set_(set) {} + using ChoiceList = std::vector<std::unique_ptr<const PropValue>>; + + explicit ConstraintOneOf(InheritableAttribute<ChoiceList> set); + explicit ConstraintOneOf(ChoiceList set); // Implementation of Constraint::GetType(). ConstraintType GetType() const override { @@ -315,46 +314,24 @@ // Implementation of Constraint::Validate(). bool Validate(const PropValue& value, - chromeos::ErrorPtr* error) const override { - using chromeos::string_utils::ToString; - T v = value.GetValueAsAny().Get<T>(); - for (const auto& item : set_.value) { - if (CompareValue(v, item)) - return true; - } - std::vector<std::string> values; - values.reserve(set_.value.size()); - for (const auto& item : set_.value) { - values.push_back(ToString(item)); - } - return ReportErrorNotOneOf(error, ToString(v), values); - } + chromeos::ErrorPtr* error) const override; // Implementation of Constraint::Clone(). - std::unique_ptr<Constraint> Clone() const override { - return std::unique_ptr<Constraint>{new ConstraintOneOf{set_}}; - } + std::unique_ptr<Constraint> Clone() const override; // Implementation of Constraint::CloneAsInherited(). - std::unique_ptr<Constraint> CloneAsInherited() const override { - return std::unique_ptr<Constraint>{new ConstraintOneOf{set_.value}}; - } + std::unique_ptr<Constraint> CloneAsInherited() const override; // Implementation of Constraint::ToJson(). - std::unique_ptr<base::Value> ToJson( - chromeos::ErrorPtr* error) const override { - return TypedValueToJson(set_.value, error); - } + std::unique_ptr<base::Value> ToJson(chromeos::ErrorPtr* error) const override; // Implementation of Constraint::GetDictKey(). - const char* GetDictKey() const override { - return commands::attributes::kOneOf_Enum; - } + const char* GetDictKey() const override; // Stores the list of acceptable values for the parameter. // |set_.is_inherited| indicates whether the constraint is inherited // from base schema or overridden. - InheritableAttribute<std::vector<T>> set_; + InheritableAttribute<ChoiceList> set_; private: DISALLOW_COPY_AND_ASSIGN(ConstraintOneOf);
diff --git a/buffet/commands/prop_types.cc b/buffet/commands/prop_types.cc index 53beb4f..3bb0577 100644 --- a/buffet/commands/prop_types.cc +++ b/buffet/commands/prop_types.cc
@@ -298,16 +298,17 @@ "Expecting an array"); return constraint; } - std::vector<T> set; - set.reserve(list->GetSize()); + ConstraintOneOf::ChoiceList choice_list; + choice_list.reserve(list->GetSize()); for (const base::Value* item : *list) { - T val{}; - if (!TypedValueFromJson(item, prop_type, &val, error)) + std::unique_ptr<PropValue> prop_value = prop_type->CreateValue(); + if (!prop_value->FromJson(item, error)) return constraint; - set.push_back(val); + choice_list.push_back(std::move(prop_value)); } - InheritableAttribute<std::vector<T>> val(set, false); - constraint.reset(new ConstraintOneOf<T>{val}); + InheritableAttribute<ConstraintOneOf::ChoiceList> val(std::move(choice_list), + false); + constraint.reset(new ConstraintOneOf{std::move(val)}); return constraint; }
diff --git a/buffet/commands/prop_types.h b/buffet/commands/prop_types.h index 892a730..1af057c 100644 --- a/buffet/commands/prop_types.h +++ b/buffet/commands/prop_types.h
@@ -200,13 +200,6 @@ bool ConstraintsFromJson(const base::DictionaryValue* value, std::set<std::string>* processed_keys, chromeos::ErrorPtr* error) override; - - // Helper method to obtain a vector of OneOf constraint values. - std::vector<T> GetOneOfValues() const { - auto ofc = static_cast<const ConstraintOneOf<T>*>( - this->GetConstraint(ConstraintType::OneOf)); - return ofc ? ofc->set_.value : std::vector<T>(); - } }; // Helper base class for Int and Double parameter types.