buffet: Added error handling to PropType::CreateValue()
PropType::CreateValue(Any) used to abort if Any didn't contain
the data of expected type. Now that we are adding more and more
reliance on D-Bus transport for GCD data (commands and state),
we can no longer guarantee that the variant data passed to
CreateValue contains the expected values (since it may be not
under our control). Add the ability for CreateValue to gracefully
fail and provide necessary error information to the caller.
BUG=chromium:415364
TEST=FEATURES=test emerge-link buffet
Change-Id: I097d674073fc84a69184a78edf2b7381608c000f
Reviewed-on: https://chromium-review.googlesource.com/219135
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_instance_unittest.cc b/buffet/commands/command_instance_unittest.cc
index 844ac92..1afd8c7 100644
--- a/buffet/commands/command_instance_unittest.cc
+++ b/buffet/commands/command_instance_unittest.cc
@@ -65,8 +65,8 @@
buffet::native_types::Object params;
buffet::IntPropType int_prop;
buffet::DoublePropType dbl_prop;
- params.insert(std::make_pair("freq", dbl_prop.CreateValue(800.5)));
- params.insert(std::make_pair("volume", int_prop.CreateValue(100)));
+ params.insert(std::make_pair("freq", dbl_prop.CreateValue(800.5, nullptr)));
+ params.insert(std::make_pair("volume", int_prop.CreateValue(100, nullptr)));
buffet::CommandInstance instance("robot._beep", "robotd", params);
EXPECT_EQ("", instance.GetID());
diff --git a/buffet/commands/object_schema_unittest.cc b/buffet/commands/object_schema_unittest.cc
index e2e32e1..e802a67 100644
--- a/buffet/commands/object_schema_unittest.cc
+++ b/buffet/commands/object_schema_unittest.cc
@@ -14,6 +14,7 @@
#include "buffet/commands/object_schema.h"
#include "buffet/commands/prop_types.h"
+#include "buffet/commands/schema_constants.h"
#include "buffet/commands/unittest_utils.h"
using buffet::unittests::CreateValue;
@@ -124,6 +125,20 @@
EXPECT_EQ("type_mismatch", error->GetCode());
}
+TEST(CommandSchema, IntPropType_CreateValue) {
+ buffet::IntPropType prop;
+ chromeos::ErrorPtr error;
+ auto val = prop.CreateValue(2, &error);
+ ASSERT_NE(nullptr, val.get());
+ EXPECT_EQ(nullptr, error.get());
+ EXPECT_EQ(2, val->GetValueAsAny().Get<int>());
+
+ val = prop.CreateValue("blah", &error);
+ EXPECT_EQ(nullptr, val.get());
+ ASSERT_NE(nullptr, error.get());
+ EXPECT_EQ(buffet::errors::commands::kTypeMismatch, error->GetCode());
+}
+
///////////////////////////////////////////////////////////////////////////////
TEST(CommandSchema, BoolPropType_Empty) {
@@ -188,6 +203,20 @@
EXPECT_EQ("type_mismatch", error->GetCode());
}
+TEST(CommandSchema, BoolPropType_CreateValue) {
+ buffet::BooleanPropType prop;
+ chromeos::ErrorPtr error;
+ auto val = prop.CreateValue(true, &error);
+ ASSERT_NE(nullptr, val.get());
+ EXPECT_EQ(nullptr, error.get());
+ EXPECT_TRUE(val->GetValueAsAny().Get<bool>());
+
+ val = prop.CreateValue("blah", &error);
+ EXPECT_EQ(nullptr, val.get());
+ ASSERT_NE(nullptr, error.get());
+ EXPECT_EQ(buffet::errors::commands::kTypeMismatch, error->GetCode());
+}
+
///////////////////////////////////////////////////////////////////////////////
TEST(CommandSchema, DoublePropType_Empty) {
@@ -285,6 +314,20 @@
EXPECT_EQ("type_mismatch", error->GetCode());
}
+TEST(CommandSchema, DoublePropType_CreateValue) {
+ buffet::DoublePropType prop;
+ chromeos::ErrorPtr error;
+ auto val = prop.CreateValue(2.0, &error);
+ ASSERT_NE(nullptr, val.get());
+ EXPECT_EQ(nullptr, error.get());
+ EXPECT_DOUBLE_EQ(2.0, val->GetValueAsAny().Get<double>());
+
+ val = prop.CreateValue("blah", &error);
+ EXPECT_EQ(nullptr, val.get());
+ ASSERT_NE(nullptr, error.get());
+ EXPECT_EQ(buffet::errors::commands::kTypeMismatch, error->GetCode());
+}
+
///////////////////////////////////////////////////////////////////////////////
TEST(CommandSchema, StringPropType_Empty) {
@@ -388,6 +431,20 @@
error.reset();
}
+TEST(CommandSchema, StringPropType_CreateValue) {
+ buffet::StringPropType prop;
+ chromeos::ErrorPtr error;
+ auto val = prop.CreateValue(std::string{"blah"}, &error);
+ ASSERT_NE(nullptr, val.get());
+ EXPECT_EQ(nullptr, error.get());
+ EXPECT_EQ("blah", val->GetValueAsAny().Get<std::string>());
+
+ val = prop.CreateValue(4, &error);
+ EXPECT_EQ(nullptr, val.get());
+ ASSERT_NE(nullptr, error.get());
+ EXPECT_EQ(buffet::errors::commands::kTypeMismatch, error->GetCode());
+}
+
///////////////////////////////////////////////////////////////////////////////
TEST(CommandSchema, ObjectPropType_Empty) {
@@ -501,6 +558,30 @@
error.reset();
}
+TEST(CommandSchema, ObjectPropType_CreateValue) {
+ buffet::ObjectPropType prop;
+ buffet::IntPropType int_type;
+ ASSERT_TRUE(prop.FromJson(CreateDictionaryValue(
+ "{'properties':{'width':'integer','height':'integer'},"
+ "'enum':[{'width':10,'height':20},{'width':100,'height':200}]}").get(),
+ nullptr, nullptr));
+ buffet::native_types::Object obj{
+ {"width", int_type.CreateValue(10, nullptr)},
+ {"height", int_type.CreateValue(20, nullptr)},
+ };
+
+ chromeos::ErrorPtr error;
+ auto val = prop.CreateValue(obj, &error);
+ ASSERT_NE(nullptr, val.get());
+ EXPECT_EQ(nullptr, error.get());
+ EXPECT_EQ(obj, val->GetValueAsAny().Get<buffet::native_types::Object>());
+
+ val = prop.CreateValue("blah", &error);
+ EXPECT_EQ(nullptr, val.get());
+ ASSERT_NE(nullptr, error.get());
+ EXPECT_EQ(buffet::errors::commands::kTypeMismatch, error->GetCode());
+}
+
///////////////////////////////////////////////////////////////////////////////
TEST(CommandSchema, ObjectSchema_FromJson_Shorthand_TypeName) {
diff --git a/buffet/commands/prop_types.cc b/buffet/commands/prop_types.cc
index 81c9546..08410cc 100644
--- a/buffet/commands/prop_types.cc
+++ b/buffet/commands/prop_types.cc
@@ -152,9 +152,8 @@
bool PropType::ValidateValue(const chromeos::Any& value,
chromeos::ErrorPtr* error) const {
- std::shared_ptr<PropValue> val = CreateValue(value);
- CHECK(val) << "Failed to create value object";
- return ValidateConstraints(*val, error);
+ std::shared_ptr<PropValue> val = CreateValue(value, error);
+ return val && ValidateConstraints(*val, error);
}
bool PropType::ValidateConstraints(const PropValue& value,
@@ -218,6 +217,14 @@
return std::unique_ptr<PropType>(prop);
}
+bool PropType::GenerateErrorValueTypeMismatch(chromeos::ErrorPtr* error) const {
+ chromeos::Error::AddToPrintf(error, errors::commands::kDomain,
+ errors::commands::kTypeMismatch,
+ "Unable to convert value to type '%s'",
+ GetTypeAsString().c_str());
+ return false;
+}
+
template<typename T>
static std::shared_ptr<Constraint> LoadOneOfConstraint(
const base::DictionaryValue* value, const ObjectSchema* object_schema,
diff --git a/buffet/commands/prop_types.h b/buffet/commands/prop_types.h
index 3320202..2533c34 100644
--- a/buffet/commands/prop_types.h
+++ b/buffet/commands/prop_types.h
@@ -83,7 +83,7 @@
// type as a factory class.
virtual std::shared_ptr<PropValue> CreateValue() const = 0;
virtual std::shared_ptr<PropValue> CreateValue(
- const chromeos::Any& val) const = 0;
+ const chromeos::Any& val, chromeos::ErrorPtr* error) const = 0;
// Saves the parameter type definition as a JSON object.
// If |full_schema| is set to true, the full type definition is saved,
@@ -163,6 +163,10 @@
bool ValidateConstraints(const PropValue& value,
chromeos::ErrorPtr* error) const;
+ // Helper method to generate "type mismatch" error when creating a value
+ // from this type. Always returns false.
+ bool GenerateErrorValueTypeMismatch(chromeos::ErrorPtr* error) const;
+
protected:
// Specifies if this parameter definition is derived from a base
// object schema.
@@ -190,10 +194,16 @@
return std::make_shared<Value>(this);
}
std::shared_ptr<PropValue> CreateValue(
- const chromeos::Any& v) const override {
- auto value = std::make_shared<Value>(this);
- value->SetValue(v.Get<T>());
- return std::move(value);
+ const chromeos::Any& v, chromeos::ErrorPtr* error) const override {
+ std::shared_ptr<PropValue> prop_value;
+ if (v.IsTypeCompatible<T>()) {
+ auto value = std::make_shared<Value>(this);
+ value->SetValue(v.Get<T>());
+ prop_value = std::move(value);
+ } else {
+ GenerateErrorValueTypeMismatch(error);
+ }
+ return prop_value;
}
bool ConstraintsFromJson(const base::DictionaryValue* value,
std::set<std::string>* processed_keys,
diff --git a/buffet/commands/schema_utils_unittest.cc b/buffet/commands/schema_utils_unittest.cc
index 1a92d7f..f3279be 100644
--- a/buffet/commands/schema_utils_unittest.cc
+++ b/buffet/commands/schema_utils_unittest.cc
@@ -56,8 +56,8 @@
buffet::IntPropType int_type;
buffet::native_types::Object object;
- object.insert(std::make_pair("width", int_type.CreateValue(640)));
- object.insert(std::make_pair("height", int_type.CreateValue(480)));
+ object.insert(std::make_pair("width", int_type.CreateValue(640, nullptr)));
+ object.insert(std::make_pair("height", int_type.CreateValue(480, nullptr)));
EXPECT_EQ("{'height':480,'width':640}",
ValueToString(buffet::TypedValueToJson(object, nullptr).get()));
}
@@ -167,9 +167,10 @@
EXPECT_TRUE(buffet::TypedValueFromJson(
CreateValue("{'age':20,'name':'Bob'}").get(), &schema, &value, nullptr));
buffet::native_types::Object value2;
- value2.insert(std::make_pair("age", age_prop->CreateValue(20)));
+ value2.insert(std::make_pair("age", age_prop->CreateValue(20, nullptr)));
value2.insert(std::make_pair("name",
- name_prop->CreateValue(std::string("Bob"))));
+ name_prop->CreateValue(std::string("Bob"),
+ nullptr)));
EXPECT_EQ(value2, value);
chromeos::ErrorPtr error;
