libweave: Cleanup creation of PropValues
Hide some constructors and test only methods.
BUG=brillo:1246
TEST='FEATURES=test emerge-gizmo buffet'
Change-Id: I1343809f94999795f8f162e3464ac22413b0dbd5
Reviewed-on: https://chromium-review.googlesource.com/289645
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Tested-by: Vitaly Buka <vitalybuka@chromium.org>
diff --git a/libweave/src/commands/object_schema_unittest.cc b/libweave/src/commands/object_schema_unittest.cc
index 018c28f..fc04dd5 100644
--- a/libweave/src/commands/object_schema_unittest.cc
+++ b/libweave/src/commands/object_schema_unittest.cc
@@ -47,6 +47,13 @@
return GetArrayValues<T>(one_of->set_.value);
}
+bool ValidateValue(const PropType& type,
+ const base::Value& value,
+ chromeos::ErrorPtr* error) {
+ std::unique_ptr<PropValue> val = type.CreatePropValue(value, error);
+ return val != nullptr;
+}
+
} // anonymous namespace
TEST(CommandSchema, IntPropType_Empty) {
@@ -123,31 +130,31 @@
IntPropType prop;
prop.AddMinMaxConstraint(2, 4);
chromeos::ErrorPtr error;
- EXPECT_FALSE(prop.ValidateValue(CreateValue("-1").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("-1"), &error));
EXPECT_EQ("out_of_range", error->GetCode());
error.reset();
- EXPECT_FALSE(prop.ValidateValue(CreateValue("0").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("0"), &error));
EXPECT_EQ("out_of_range", error->GetCode());
error.reset();
- EXPECT_FALSE(prop.ValidateValue(CreateValue("1").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("1"), &error));
EXPECT_EQ("out_of_range", error->GetCode());
error.reset();
- EXPECT_TRUE(prop.ValidateValue(CreateValue("2").get(), &error));
+ EXPECT_TRUE(ValidateValue(prop, *CreateValue("2"), &error));
EXPECT_EQ(nullptr, error.get());
- EXPECT_TRUE(prop.ValidateValue(CreateValue("3").get(), &error));
+ EXPECT_TRUE(ValidateValue(prop, *CreateValue("3"), &error));
EXPECT_EQ(nullptr, error.get());
- EXPECT_TRUE(prop.ValidateValue(CreateValue("4").get(), &error));
+ EXPECT_TRUE(ValidateValue(prop, *CreateValue("4"), &error));
EXPECT_EQ(nullptr, error.get());
- EXPECT_FALSE(prop.ValidateValue(CreateValue("5").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("5"), &error));
EXPECT_EQ("out_of_range", error->GetCode());
error.reset();
- EXPECT_FALSE(prop.ValidateValue(CreateValue("true").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("true"), &error));
EXPECT_EQ("type_mismatch", error->GetCode());
error.reset();
- EXPECT_FALSE(prop.ValidateValue(CreateValue("3.0").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("3.0"), &error));
EXPECT_EQ("type_mismatch", error->GetCode());
error.reset();
- EXPECT_FALSE(prop.ValidateValue(CreateValue("'3'").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("'3'"), &error));
EXPECT_EQ("type_mismatch", error->GetCode());
}
@@ -225,18 +232,18 @@
BooleanPropType prop;
prop.FromJson(CreateDictionaryValue("{'enum':[true]}").get(), &prop, nullptr);
chromeos::ErrorPtr error;
- EXPECT_FALSE(prop.ValidateValue(CreateValue("false").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("false"), &error));
EXPECT_EQ("out_of_range", error->GetCode());
error.reset();
- EXPECT_TRUE(prop.ValidateValue(CreateValue("true").get(), &error));
+ EXPECT_TRUE(ValidateValue(prop, *CreateValue("true"), &error));
error.reset();
- EXPECT_FALSE(prop.ValidateValue(CreateValue("1").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("1"), &error));
EXPECT_EQ("type_mismatch", error->GetCode());
error.reset();
- EXPECT_FALSE(prop.ValidateValue(CreateValue("3.0").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("3.0"), &error));
EXPECT_EQ("type_mismatch", error->GetCode());
error.reset();
- EXPECT_FALSE(prop.ValidateValue(CreateValue("'3'").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("'3'"), &error));
EXPECT_EQ("type_mismatch", error->GetCode());
}
@@ -335,25 +342,25 @@
DoublePropType prop;
prop.AddMinMaxConstraint(-1.2, 1.3);
chromeos::ErrorPtr error;
- EXPECT_FALSE(prop.ValidateValue(CreateValue("-2").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("-2"), &error));
EXPECT_EQ("out_of_range", error->GetCode());
error.reset();
- EXPECT_FALSE(prop.ValidateValue(CreateValue("-1.3").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("-1.3"), &error));
EXPECT_EQ("out_of_range", error->GetCode());
error.reset();
- EXPECT_TRUE(prop.ValidateValue(CreateValue("-1.2").get(), &error));
+ EXPECT_TRUE(ValidateValue(prop, *CreateValue("-1.2"), &error));
EXPECT_EQ(nullptr, error.get());
- EXPECT_TRUE(prop.ValidateValue(CreateValue("0.0").get(), &error));
+ EXPECT_TRUE(ValidateValue(prop, *CreateValue("0.0"), &error));
EXPECT_EQ(nullptr, error.get());
- EXPECT_TRUE(prop.ValidateValue(CreateValue("1.3").get(), &error));
+ EXPECT_TRUE(ValidateValue(prop, *CreateValue("1.3"), &error));
EXPECT_EQ(nullptr, error.get());
- EXPECT_FALSE(prop.ValidateValue(CreateValue("1.31").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("1.31"), &error));
EXPECT_EQ("out_of_range", error->GetCode());
error.reset();
- EXPECT_FALSE(prop.ValidateValue(CreateValue("true").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("true"), &error));
EXPECT_EQ("type_mismatch", error->GetCode());
error.reset();
- EXPECT_FALSE(prop.ValidateValue(CreateValue("'0.0'").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("'0.0'"), &error));
EXPECT_EQ("type_mismatch", error->GetCode());
}
@@ -454,30 +461,30 @@
StringPropType prop;
prop.AddLengthConstraint(1, 3);
chromeos::ErrorPtr error;
- EXPECT_FALSE(prop.ValidateValue(CreateValue("''").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("''"), &error));
EXPECT_EQ("out_of_range", error->GetCode());
error.reset();
prop.AddLengthConstraint(2, 3);
- EXPECT_FALSE(prop.ValidateValue(CreateValue("''").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("''"), &error));
EXPECT_EQ("out_of_range", error->GetCode());
error.reset();
- EXPECT_FALSE(prop.ValidateValue(CreateValue("'a'").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("'a'"), &error));
EXPECT_EQ("out_of_range", error->GetCode());
error.reset();
- EXPECT_TRUE(prop.ValidateValue(CreateValue("'ab'").get(), &error));
+ EXPECT_TRUE(ValidateValue(prop, *CreateValue("'ab'"), &error));
EXPECT_EQ(nullptr, error.get());
- EXPECT_TRUE(prop.ValidateValue(CreateValue("'abc'").get(), &error));
+ EXPECT_TRUE(ValidateValue(prop, *CreateValue("'abc'"), &error));
EXPECT_EQ(nullptr, error.get());
- EXPECT_FALSE(prop.ValidateValue(CreateValue("'abcd'").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("'abcd'"), &error));
EXPECT_EQ("out_of_range", error->GetCode());
error.reset();
prop.FromJson(CreateDictionaryValue("{'enum':['abc','def','xyz!!']}").get(),
nullptr, &error);
- EXPECT_TRUE(prop.ValidateValue(CreateValue("'abc'").get(), &error));
- EXPECT_TRUE(prop.ValidateValue(CreateValue("'def'").get(), &error));
- EXPECT_TRUE(prop.ValidateValue(CreateValue("'xyz!!'").get(), &error));
- EXPECT_FALSE(prop.ValidateValue(CreateValue("'xyz'").get(), &error));
+ EXPECT_TRUE(ValidateValue(prop, *CreateValue("'abc'"), &error));
+ EXPECT_TRUE(ValidateValue(prop, *CreateValue("'def'"), &error));
+ EXPECT_TRUE(ValidateValue(prop, *CreateValue("'xyz!!'"), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("'xyz'"), &error));
EXPECT_EQ("out_of_range", error->GetCode());
error.reset();
}
@@ -676,30 +683,30 @@
"'required':['expires','password']}").get(),
nullptr, nullptr);
chromeos::ErrorPtr error;
- EXPECT_TRUE(prop.ValidateValue(
- CreateValue("{'expires':10,'password':'abcdef'}").get(), &error));
+ EXPECT_TRUE(ValidateValue(
+ prop, *CreateValue("{'expires':10,'password':'abcdef'}"), &error));
error.reset();
- EXPECT_FALSE(prop.ValidateValue(CreateValue("{'expires':10}").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("{'expires':10}"), &error));
EXPECT_EQ("parameter_missing", error->GetCode());
error.reset();
EXPECT_FALSE(
- prop.ValidateValue(CreateValue("{'password':'abcdef'}").get(), &error));
+ ValidateValue(prop, *CreateValue("{'password':'abcdef'}"), &error));
EXPECT_EQ("parameter_missing", error->GetCode());
error.reset();
- EXPECT_FALSE(prop.ValidateValue(
- CreateValue("{'expires':10,'password':'abcde'}").get(), &error));
+ EXPECT_FALSE(ValidateValue(
+ prop, *CreateValue("{'expires':10,'password':'abcde'}"), &error));
EXPECT_EQ("out_of_range", error->GetFirstError()->GetCode());
error.reset();
- EXPECT_FALSE(prop.ValidateValue(CreateValue("2").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("2"), &error));
EXPECT_EQ("type_mismatch", error->GetCode());
error.reset();
- EXPECT_FALSE(prop.ValidateValue(
- CreateValue("{'expires':10,'password':'abcdef','retry':true}").get(),
+ EXPECT_FALSE(ValidateValue(
+ prop, *CreateValue("{'expires':10,'password':'abcdef','retry':true}"),
&error));
EXPECT_EQ("unexpected_parameter", error->GetCode());
error.reset();
@@ -714,16 +721,16 @@
.get(),
nullptr, nullptr));
chromeos::ErrorPtr error;
- EXPECT_TRUE(prop.ValidateValue(CreateValue("{'height':20,'width':10}").get(),
- &error));
+ EXPECT_TRUE(
+ ValidateValue(prop, *CreateValue("{'height':20,'width':10}"), &error));
error.reset();
- EXPECT_TRUE(prop.ValidateValue(
- CreateValue("{'height':200,'width':100}").get(), &error));
+ EXPECT_TRUE(
+ ValidateValue(prop, *CreateValue("{'height':200,'width':100}"), &error));
error.reset();
- EXPECT_FALSE(prop.ValidateValue(CreateValue("{'height':12,'width':10}").get(),
- &error));
+ EXPECT_FALSE(
+ ValidateValue(prop, *CreateValue("{'height':12,'width':10}"), &error));
EXPECT_EQ("out_of_range", error->GetCode());
error.reset();
}
@@ -825,14 +832,14 @@
nullptr, nullptr);
chromeos::ErrorPtr error;
- EXPECT_TRUE(prop.ValidateValue(CreateValue("[3,4,10.5]").get(), &error));
+ EXPECT_TRUE(ValidateValue(prop, *CreateValue("[3,4,10.5]"), &error));
error.reset();
- EXPECT_FALSE(prop.ValidateValue(CreateValue("[2]").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("[2]"), &error));
EXPECT_EQ("out_of_range", error->GetCode());
error.reset();
- EXPECT_FALSE(prop.ValidateValue(CreateValue("[4, 5, 20]").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("[4, 5, 20]"), &error));
EXPECT_EQ("out_of_range", error->GetCode());
error.reset();
}
@@ -845,14 +852,14 @@
nullptr, nullptr);
chromeos::ErrorPtr error;
- EXPECT_TRUE(prop.ValidateValue(CreateValue("[2,3]").get(), &error));
+ EXPECT_TRUE(ValidateValue(prop, *CreateValue("[2,3]"), &error));
error.reset();
- EXPECT_FALSE(prop.ValidateValue(CreateValue("[2]").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("[2]"), &error));
EXPECT_EQ("out_of_range", error->GetCode());
error.reset();
- EXPECT_FALSE(prop.ValidateValue(CreateValue("[2,3,4]").get(), &error));
+ EXPECT_FALSE(ValidateValue(prop, *CreateValue("[2,3,4]"), &error));
EXPECT_EQ("out_of_range", error->GetCode());
error.reset();
}
@@ -1267,8 +1274,9 @@
prop.FromJson(CreateDictionaryValue(schema_str).get(), nullptr, nullptr));
// Omit all.
- auto value = prop.CreateValue();
- ASSERT_TRUE(value->FromJson(CreateDictionaryValue("{}").get(), nullptr));
+ auto value =
+ prop.CreatePropValue(*CreateDictionaryValue("{}").get(), nullptr);
+ ASSERT_NE(nullptr, value);
ValueMap obj = value->GetObject()->GetValue();
EXPECT_TRUE(obj["param1"]->GetBoolean()->GetValue());
EXPECT_EQ(2, obj["param2"]->GetInt()->GetValue());
@@ -1281,14 +1289,14 @@
EXPECT_EQ((std::vector<int>{1, 2, 3}), GetArrayValues<int>(param6));
// Specify some.
- value = prop.CreateValue();
const char* val_json =
"{"
"'param1':false,"
"'param3':33.3,"
"'param5':{'x':-5,'y':-6}"
"}";
- ASSERT_TRUE(value->FromJson(CreateDictionaryValue(val_json).get(), nullptr));
+ value = prop.CreatePropValue(*CreateDictionaryValue(val_json).get(), nullptr);
+ ASSERT_NE(nullptr, value);
obj = value->GetObject()->GetValue();
EXPECT_FALSE(obj["param1"]->GetBoolean()->GetValue());
EXPECT_EQ(2, obj["param2"]->GetInt()->GetValue());
@@ -1301,7 +1309,6 @@
EXPECT_EQ((std::vector<int>{1, 2, 3}), GetArrayValues<int>(param6));
// Specify all.
- value = prop.CreateValue();
val_json =
"{"
"'param1':false,"
@@ -1311,7 +1318,8 @@
"'param5':{'x':-55,'y':66},"
"'param6':[-1, 0]"
"}";
- ASSERT_TRUE(value->FromJson(CreateDictionaryValue(val_json).get(), nullptr));
+ value = prop.CreatePropValue(*CreateDictionaryValue(val_json).get(), nullptr);
+ ASSERT_NE(nullptr, value);
obj = value->GetObject()->GetValue();
EXPECT_FALSE(obj["param1"]->GetBoolean()->GetValue());
EXPECT_EQ(22, obj["param2"]->GetInt()->GetValue());
@@ -1638,23 +1646,24 @@
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));
+ auto value =
+ prop.CreatePropValue(*CreateDictionaryValue(val_json).get(), nullptr);
+ ASSERT_NE(nullptr, value);
ValueMap 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));
+ value = prop.CreatePropValue(*CreateDictionaryValue(val_json).get(), nullptr);
+ ASSERT_NE(nullptr, value);
obj = value->GetObject()->GetValue();
EXPECT_EQ(3, obj.size());
@@ -1678,10 +1687,11 @@
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));
+ auto value =
+ prop.CreatePropValue(*CreateDictionaryValue(val_json).get(), &error);
+ ASSERT_EQ(nullptr, value);
EXPECT_EQ(errors::commands::kPropertyMissing, error->GetCode());
}
diff --git a/libweave/src/commands/prop_types.cc b/libweave/src/commands/prop_types.cc
index 4ddfc00..6daabc0 100644
--- a/libweave/src/commands/prop_types.cc
+++ b/libweave/src/commands/prop_types.cc
@@ -185,8 +185,8 @@
// so we can parse and validate the value of the default.
const base::Value* defval = nullptr; // Owned by value
if (value->GetWithoutPathExpansion(commands::attributes::kDefault, &defval)) {
- std::unique_ptr<PropValue> prop_value = CreateValue();
- if (!prop_value->FromJson(defval, error)) {
+ std::unique_ptr<PropValue> prop_value = CreatePropValue(*defval, error);
+ if (!prop_value) {
chromeos::Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain,
errors::commands::kInvalidPropValue,
"Invalid value for property '%s'",
@@ -231,13 +231,6 @@
return p != constraints_.end() ? p->second.get() : nullptr;
}
-bool PropType::ValidateValue(const base::Value* value,
- chromeos::ErrorPtr* error) const {
- std::unique_ptr<PropValue> val = CreateValue();
- CHECK(val) << "Failed to create value object";
- return val->FromJson(value, error);
-}
-
bool PropType::ValidateConstraints(const PropValue& value,
chromeos::ErrorPtr* error) const {
for (const auto& pair : constraints_) {
diff --git a/libweave/src/commands/prop_types.h b/libweave/src/commands/prop_types.h
index 71de94d..c8b1338 100644
--- a/libweave/src/commands/prop_types.h
+++ b/libweave/src/commands/prop_types.h
@@ -85,9 +85,10 @@
// Makes a full copy of this type definition.
virtual std::unique_ptr<PropType> Clone() const;
- // Creates an instance of associated value object, using the parameter
- // type as a factory class.
- virtual std::unique_ptr<PropValue> CreateValue() const = 0;
+ // Creates an instance of associated value object.
+ virtual std::unique_ptr<PropValue> CreatePropValue(
+ const base::Value& value,
+ 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,
@@ -125,13 +126,6 @@
return true;
}
- // Validates a JSON value for the parameter type to make sure it satisfies
- // the parameter type definition including any specified constraints.
- // Returns false if the |value| does not meet the requirements of the type
- // definition and returns additional information about the failure via
- // the |error| parameter.
- bool ValidateValue(const base::Value* value, chromeos::ErrorPtr* error) const;
-
// Additional helper static methods to help with converting a type enum
// value into a string and back.
using TypeMap = std::vector<std::pair<ValueType, std::string>>;
@@ -162,6 +156,9 @@
chromeos::ErrorPtr* error) const;
protected:
+ friend class StatePackage;
+ virtual std::unique_ptr<PropValue> CreateDefaultValue() const = 0;
+
// Specifies if this parameter definition is derived from a base
// object schema.
bool based_on_schema_ = false;
@@ -188,23 +185,27 @@
// Overrides from PropType.
ValueType GetType() const override { return GetValueType<T>(); }
- std::unique_ptr<PropValue> CreateValue() const override {
- if (GetDefaultValue())
- return GetDefaultValue()->Clone();
- return std::unique_ptr<PropValue>{new Value{Clone()}};
+ std::unique_ptr<PropValue> CreatePropValue(
+ const base::Value& value,
+ chromeos::ErrorPtr* error) const override {
+ return CreateValue(value, error);
}
std::unique_ptr<Value> CreateValue(const base::Value& value,
chromeos::ErrorPtr* error) const {
- std::unique_ptr<Value> prop_value{new Value{Clone()}};
- if (!prop_value->FromJson(&value, error))
- return nullptr;
- return prop_value;
+ return Value::CreateFromJson(value, *this, error);
}
bool ConstraintsFromJson(const base::DictionaryValue* value,
std::set<std::string>* processed_keys,
chromeos::ErrorPtr* error) override;
+
+ protected:
+ std::unique_ptr<PropValue> CreateDefaultValue() const override {
+ if (GetDefaultValue())
+ return GetDefaultValue()->Clone();
+ return std::unique_ptr<PropValue>{new Value{*this}};
+ }
};
// Helper base class for Int and Double parameter types.
diff --git a/libweave/src/commands/prop_values.cc b/libweave/src/commands/prop_values.cc
index 813f9c6..cc70948 100644
--- a/libweave/src/commands/prop_values.cc
+++ b/libweave/src/commands/prop_values.cc
@@ -8,12 +8,9 @@
namespace weave {
-PropValue::PropValue(std::unique_ptr<const PropType> type)
- : type_{std::move(type)} {
-}
+PropValue::PropValue(const PropType& type) : type_{type.Clone()} {}
-PropValue::PropValue(const PropValue& other)
- : PropValue{other.type_->Clone()} {}
+PropValue::PropValue(const PropValue& other) : PropValue{*other.type_} {}
PropValue::~PropValue() {
}
diff --git a/libweave/src/commands/prop_values.h b/libweave/src/commands/prop_values.h
index d50c1a4..939356a 100644
--- a/libweave/src/commands/prop_values.h
+++ b/libweave/src/commands/prop_values.h
@@ -83,7 +83,8 @@
// This is used to validate the values against "enum"/"one of" constraints.
class PropValue {
public:
- explicit PropValue(std::unique_ptr<const PropType> type);
+ // Only CreateDefaultValue should use this constructor.
+ explicit PropValue(const PropType& type);
virtual ~PropValue();
// Gets the type of the value.
@@ -108,11 +109,6 @@
// Saves the value as a JSON object. Never fails.
virtual std::unique_ptr<base::Value> ToJson() const = 0;
- // Parses a value from JSON.
- // If it fails, it returns false and provides additional information
- // via the |error| parameter.
- virtual bool FromJson(const base::Value* value,
- chromeos::ErrorPtr* error) = 0;
// Return the type definition of this value.
const PropType* GetPropType() const { return type_.get(); }
@@ -134,9 +130,6 @@
template <typename T>
class TypedValueBase : public PropValue {
public:
- // To help refer to this base class from derived classes, define Base to
- // be this class.
- using Base = TypedValueBase<T>;
using PropValue::PropValue;
// Overrides from PropValue base class.
@@ -146,34 +139,23 @@
return TypedValueToJson(value_);
}
- bool FromJson(const base::Value* value, chromeos::ErrorPtr* error) override {
- T tmp_value;
- if (!TypedValueFromJson(value, GetPropType(), &tmp_value, error))
- return false;
- return SetValue(tmp_value, error);
- }
-
bool IsEqual(const PropValue* value) const override {
if (GetType() != value->GetType())
return false;
- const Base* value_base = static_cast<const Base*>(value);
- return CompareValue(GetValue(), value_base->GetValue());
+ return CompareValue(GetValue(),
+ static_cast<const TypedValueBase*>(value)->GetValue());
}
// Helper methods to get and set the C++ representation of the value.
const T& GetValue() const { return value_; }
- bool SetValue(T value, chromeos::ErrorPtr* error) {
- std::swap(value_, value); // Backup.
- if (GetPropType()->ValidateConstraints(*this, error))
- return true;
- std::swap(value_, value); // Restore.
- return false;
- }
protected:
explicit TypedValueBase(const TypedValueBase& other)
: PropValue(other), value_(other.value_) {}
+ TypedValueBase(const PropType& type, T value)
+ : PropValue(type), value_(value) {}
+
private:
T value_{}; // The value of the parameter in C++ data representation.
};
@@ -192,12 +174,28 @@
return std::unique_ptr<PropValue>{
new Derived{*static_cast<const Derived*>(this)}};
}
+
+ static std::unique_ptr<Derived> CreateFromJson(const base::Value& value,
+ const PropType& type,
+ chromeos::ErrorPtr* error) {
+ T tmp_value;
+ if (!TypedValueFromJson(&value, &type, &tmp_value, error))
+ return nullptr;
+
+ // Only place where invalid value can exist.
+ std::unique_ptr<Derived> result{new Derived{type, tmp_value}};
+ if (!result->GetPropType()->ValidateConstraints(*result, error))
+ return nullptr;
+
+ return result;
+ }
};
// Value of type Integer.
class IntValue final : public TypedValueWithClone<IntValue, int> {
public:
- using Base::Base; // Expose the custom constructor of the base class.
+ using Base::Base;
+ friend class TypedValueWithClone<IntValue, int>;
IntValue* GetInt() override { return this; }
IntValue const* GetInt() const override { return this; }
};
@@ -205,7 +203,8 @@
// Value of type Number.
class DoubleValue final : public TypedValueWithClone<DoubleValue, double> {
public:
- using Base::Base; // Expose the custom constructor of the base class.
+ using Base::Base;
+ friend class TypedValueWithClone<DoubleValue, double>;
DoubleValue* GetDouble() override { return this; }
DoubleValue const* GetDouble() const override { return this; }
};
@@ -213,7 +212,8 @@
// Value of type String.
class StringValue final : public TypedValueWithClone<StringValue, std::string> {
public:
- using Base::Base; // Expose the custom constructor of the base class.
+ using Base::Base;
+ friend class TypedValueWithClone<StringValue, std::string>;
StringValue* GetString() override { return this; }
StringValue const* GetString() const override { return this; }
};
@@ -221,7 +221,8 @@
// Value of type Boolean.
class BooleanValue final : public TypedValueWithClone<BooleanValue, bool> {
public:
- using Base::Base; // Expose the custom constructor of the base class.
+ using Base::Base;
+ friend class TypedValueWithClone<BooleanValue, bool>;
BooleanValue* GetBoolean() override { return this; }
BooleanValue const* GetBoolean() const override { return this; }
};
@@ -229,7 +230,8 @@
// Value of type Object.
class ObjectValue final : public TypedValueWithClone<ObjectValue, ValueMap> {
public:
- using Base::Base; // Expose the custom constructor of the base class.
+ using Base::Base;
+ friend class TypedValueWithClone<ObjectValue, ValueMap>;
ObjectValue* GetObject() override { return this; }
ObjectValue const* GetObject() const override { return this; }
};
@@ -237,7 +239,8 @@
// Value of type Array.
class ArrayValue final : public TypedValueWithClone<ArrayValue, ValueVector> {
public:
- using Base::Base; // Expose the custom constructor of the base class.
+ using Base::Base;
+ friend class TypedValueWithClone<ArrayValue, ValueVector>;
ArrayValue* GetArray() override { return this; }
ArrayValue const* GetArray() const override { return this; }
};
diff --git a/libweave/src/commands/schema_utils.cc b/libweave/src/commands/schema_utils.cc
index 034eca9..e2c8293 100644
--- a/libweave/src/commands/schema_utils.cc
+++ b/libweave/src/commands/schema_utils.cc
@@ -143,11 +143,11 @@
for (const auto& pair : object_schema->GetProps()) {
const PropValue* def_value = pair.second->GetDefaultValue();
if (dict->HasKey(pair.first)) {
- auto value = pair.second->CreateValue();
const base::Value* param_value = nullptr;
CHECK(dict->GetWithoutPathExpansion(pair.first, ¶m_value))
<< "Unable to get parameter";
- if (!value->FromJson(param_value, error)) {
+ auto value = pair.second->CreatePropValue(*param_value, error);
+ if (!value) {
chromeos::Error::AddToPrintf(
error, FROM_HERE, errors::commands::kDomain,
errors::commands::kInvalidPropValue,
@@ -213,10 +213,10 @@
value_out->clear();
value_out->reserve(list->GetSize());
for (const base::Value* item : *list) {
- std::unique_ptr<PropValue> prop_value = item_type->CreateValue();
- if (!prop_value->FromJson(item, error)) {
+ std::unique_ptr<PropValue> prop_value =
+ item_type->CreatePropValue(*item, error);
+ if (!prop_value)
return false;
- }
value_out->push_back(std::move(prop_value));
}
return true;
diff --git a/libweave/src/commands/unittest_utils.h b/libweave/src/commands/unittest_utils.h
index d0d39ee..043bace 100644
--- a/libweave/src/commands/unittest_utils.h
+++ b/libweave/src/commands/unittest_utils.h
@@ -30,30 +30,27 @@
return val1.Equals(&val2);
}
-template <typename PropVal, typename T>
-std::unique_ptr<const PropVal> make_prop_value(const T& value) {
- std::unique_ptr<PropVal> result{
- new PropVal{PropType::Create(GetValueType<T>())}};
- if (!result->SetValue(value, nullptr))
- return nullptr;
- return std::move(result);
+template <typename T>
+std::unique_ptr<const PropValue> make_prop_value(const base::Value& value) {
+ auto prop_type = PropType::Create(GetValueType<T>());
+ return prop_type->CreatePropValue(value, nullptr);
}
-inline std::unique_ptr<const IntValue> make_int_prop_value(int value) {
- return make_prop_value<IntValue, int>(value);
+inline std::unique_ptr<const PropValue> make_int_prop_value(int value) {
+ return make_prop_value<int>(base::FundamentalValue{value});
}
-inline std::unique_ptr<const DoubleValue> make_double_prop_value(double value) {
- return make_prop_value<DoubleValue, double>(value);
+inline std::unique_ptr<const PropValue> make_double_prop_value(double value) {
+ return make_prop_value<double>(base::FundamentalValue{value});
}
-inline std::unique_ptr<const BooleanValue> make_bool_prop_value(bool value) {
- return make_prop_value<BooleanValue, bool>(value);
+inline std::unique_ptr<const PropValue> make_bool_prop_value(bool value) {
+ return make_prop_value<bool>(base::FundamentalValue{value});
}
-inline std::unique_ptr<const StringValue> make_string_prop_value(
+inline std::unique_ptr<const PropValue> make_string_prop_value(
const std::string& value) {
- return make_prop_value<StringValue, std::string>(value);
+ return make_prop_value<std::string>(base::StringValue{value});
}
} // namespace unittests
diff --git a/libweave/src/states/state_package.cc b/libweave/src/states/state_package.cc
index 170b03c..e20d267 100644
--- a/libweave/src/states/state_package.cc
+++ b/libweave/src/states/state_package.cc
@@ -38,7 +38,7 @@
for (const auto& pair : schema.GetProps()) {
types_.AddProp(pair.first, pair.second->Clone());
// Create default value for this state property.
- values_.emplace(pair.first, pair.second->CreateValue());
+ values_.emplace(pair.first, pair.second->CreateDefaultValue());
}
return true;
@@ -89,8 +89,8 @@
name_.c_str(), property_name.c_str());
return false;
}
- auto new_value = it->second->GetPropType()->CreateValue();
- if (!new_value->FromJson(&value, error))
+ auto new_value = it->second->GetPropType()->CreatePropValue(value, error);
+ if (!new_value)
return false;
it->second = std::move(new_value);
return true;
diff --git a/libweave/src/states/state_package_unittest.cc b/libweave/src/states/state_package_unittest.cc
index 0bd4be8..d6978c0 100644
--- a/libweave/src/states/state_package_unittest.cc
+++ b/libweave/src/states/state_package_unittest.cc
@@ -330,10 +330,17 @@
EXPECT_EQ(errors::commands::kUnknownProperty, error->GetCode());
}
+TEST_F(StatePackageTest, SetPropertyValue_Object_OptionalProperty) {
+ EXPECT_JSON_EQ("{'altitude': 89.9, 'azimuth': 57.2957795}",
+ *package_->GetProperty("direction")->ToJson());
+ ASSERT_TRUE(package_->SetPropertyValue(
+ "direction", *CreateDictionaryValue("{'azimuth': 10.0}"), nullptr));
+ EXPECT_JSON_EQ("{'azimuth': 10.0}",
+ *package_->GetProperty("direction")->ToJson());
+}
+
TEST_F(StatePackageTest, SetPropertyValue_Error_Object_MissingProperty) {
chromeos::ErrorPtr error;
- ASSERT_TRUE(package_->SetPropertyValue(
- "direction", *CreateDictionaryValue("{'azimuth': 10.0}"), &error));
ASSERT_FALSE(package_->SetPropertyValue(
"direction", *CreateDictionaryValue("{'altitude': 10.0}"), &error));
EXPECT_EQ(errors::commands::kDomain, error->GetDomain());
