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, &param_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());