buffet: Change shared ownership of types/values to exclusive
In order to support array types in Buffet's type system, PropValue
must maintain a strong reference to the underlying PropType since
the array elements will have their own type reference and the current
implementation of using raw pointer for this isn't going to work.
In order to resolve this I had to make a lot of changes to the object
ownership inside Buffet's type system. I made it possible for both
PropType and PropValue to make a deep copy of itself using their
Clone() methods. Because of this now it is possible to have exclusive
ownership of objects and I got rid of most of shared pointer usage in
ObjectSchema, PropType and PropValue.
BUG=brillo:107
TEST=`FEATURES=test emerge-link buffet`
Change-Id: I02de455dfd40d4833041b63cbb80bcb00293b5a9
Reviewed-on: https://chromium-review.googlesource.com/261336
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/command_definition.cc b/buffet/commands/command_definition.cc
index 7494de8..c5854c2 100644
--- a/buffet/commands/command_definition.cc
+++ b/buffet/commands/command_definition.cc
@@ -8,9 +8,10 @@
CommandDefinition::CommandDefinition(
const std::string& category,
- const std::shared_ptr<const ObjectSchema>& parameters,
- const std::shared_ptr<const ObjectSchema>& results)
- : category_(category), parameters_(parameters), results_(results) {
-}
+ std::unique_ptr<const ObjectSchema> parameters,
+ std::unique_ptr<const ObjectSchema> results)
+ : category_{category},
+ parameters_{std::move(parameters)},
+ results_{std::move(results)} {}
} // namespace buffet
diff --git a/buffet/commands/command_definition.h b/buffet/commands/command_definition.h
index a7b793a..0cc09b5 100644
--- a/buffet/commands/command_definition.h
+++ b/buffet/commands/command_definition.h
@@ -22,24 +22,20 @@
class CommandDefinition {
public:
CommandDefinition(const std::string& category,
- const std::shared_ptr<const ObjectSchema>& parameters,
- const std::shared_ptr<const ObjectSchema>& results);
+ std::unique_ptr<const ObjectSchema> parameters,
+ std::unique_ptr<const ObjectSchema> results);
// Gets the category this command belongs to.
const std::string& GetCategory() const { return category_; }
// Gets the object schema for command parameters.
- const std::shared_ptr<const ObjectSchema>& GetParameters() const {
- return parameters_;
- }
+ const ObjectSchema* GetParameters() const { return parameters_.get(); }
// Gets the object schema for command results.
- const std::shared_ptr<const ObjectSchema>& GetResults() const {
- return results_;
- }
+ const ObjectSchema* GetResults() const { return results_.get(); }
private:
std::string category_; // Cmd category. Could be "powerd" for "base.reboot".
- std::shared_ptr<const ObjectSchema> parameters_; // Command parameters def.
- std::shared_ptr<const ObjectSchema> results_; // Command results def.
+ std::unique_ptr<const ObjectSchema> parameters_; // Command parameters def.
+ std::unique_ptr<const ObjectSchema> results_; // Command results def.
DISALLOW_COPY_AND_ASSIGN(CommandDefinition);
};
diff --git a/buffet/commands/command_definition_unittest.cc b/buffet/commands/command_definition_unittest.cc
index 0b06086..78520f8 100644
--- a/buffet/commands/command_definition_unittest.cc
+++ b/buffet/commands/command_definition_unittest.cc
@@ -6,11 +6,16 @@
#include <gtest/gtest.h>
+using buffet::ObjectSchema;
+
TEST(CommandDefinition, Test) {
- auto params = std::make_shared<buffet::ObjectSchema>();
- auto results = std::make_shared<buffet::ObjectSchema>();
- buffet::CommandDefinition def("powerd", params, results);
+ std::unique_ptr<const ObjectSchema> params{ObjectSchema::Create()};
+ std::unique_ptr<const ObjectSchema> results{ObjectSchema::Create()};
+ const ObjectSchema* param_ptr = params.get();
+ const ObjectSchema* results_ptr = results.get();
+ buffet::CommandDefinition def{"powerd", std::move(params),
+ std::move(results)};
EXPECT_EQ("powerd", def.GetCategory());
- EXPECT_EQ(params, def.GetParameters());
- EXPECT_EQ(results, def.GetResults());
+ EXPECT_EQ(param_ptr, def.GetParameters());
+ EXPECT_EQ(results_ptr, def.GetResults());
}
diff --git a/buffet/commands/command_dictionary.cc b/buffet/commands/command_dictionary.cc
index 14bead8..68e963c 100644
--- a/buffet/commands/command_dictionary.cc
+++ b/buffet/commands/command_dictionary.cc
@@ -26,7 +26,7 @@
const std::string& category,
const CommandDictionary* base_commands,
chromeos::ErrorPtr* error) {
- std::map<std::string, std::shared_ptr<const CommandDefinition>> new_defs;
+ CommandMap new_defs;
// |json| contains a list of nested objects with the following structure:
// {"<pkg_name>": {"<cmd_name>": {"parameters": {object_schema}}, ...}, ...}
@@ -72,8 +72,8 @@
if (base_commands) {
auto cmd = base_commands->FindCommand(full_command_name);
if (cmd) {
- base_parameters_def = cmd->GetParameters().get();
- base_results_def = cmd->GetResults().get();
+ base_parameters_def = cmd->GetParameters();
+ base_results_def = cmd->GetResults();
}
// If the base command dictionary was provided but the command was not
@@ -111,10 +111,11 @@
if (!results_schema)
return false;
- auto command_def = std::make_shared<CommandDefinition>(category,
- parameters_schema,
- results_schema);
- new_defs.insert(std::make_pair(full_command_name, command_def));
+ std::unique_ptr<CommandDefinition> command_def{
+ new CommandDefinition{category, std::move(parameters_schema),
+ std::move(results_schema)}
+ };
+ new_defs.emplace(full_command_name, std::move(command_def));
command_iter.Advance();
}
@@ -140,17 +141,18 @@
definitions_.erase(name);
// Insert new definitions into the global map.
- definitions_.insert(new_defs.begin(), new_defs.end());
+ for (auto& pair : new_defs)
+ definitions_.emplace(pair.first, std::move(pair.second));
return true;
}
-std::shared_ptr<ObjectSchema> CommandDictionary::BuildObjectSchema(
+std::unique_ptr<ObjectSchema> CommandDictionary::BuildObjectSchema(
const base::DictionaryValue* command_def_json,
const char* property_name,
const ObjectSchema* base_def,
const std::string& command_name,
chromeos::ErrorPtr* error) {
- auto object_schema = std::make_shared<ObjectSchema>();
+ auto object_schema = ObjectSchema::Create();
const base::DictionaryValue* schema_def = nullptr;
if (!command_def_json->GetDictionaryWithoutPathExpansion(property_name,
@@ -204,11 +206,10 @@
return dict;
}
-std::shared_ptr<const CommandDefinition> CommandDictionary::FindCommand(
+const CommandDefinition* CommandDictionary::FindCommand(
const std::string& command_name) const {
auto pair = definitions_.find(command_name);
- return (pair != definitions_.end()) ? pair->second :
- std::shared_ptr<const CommandDefinition>();
+ return (pair != definitions_.end()) ? pair->second.get() : nullptr;
}
void CommandDictionary::Clear() {
diff --git a/buffet/commands/command_dictionary.h b/buffet/commands/command_dictionary.h
index d051c62..414d79a 100644
--- a/buffet/commands/command_dictionary.h
+++ b/buffet/commands/command_dictionary.h
@@ -13,6 +13,8 @@
#include <base/macros.h>
#include <chromeos/errors/error.h>
+#include "buffet/commands/command_definition.h"
+
namespace base {
class Value;
class DictionaryValue;
@@ -20,7 +22,6 @@
namespace buffet {
-class CommandDefinition;
class ObjectSchema;
// CommandDictionary is a wrapper around a map of command name and the
@@ -69,20 +70,19 @@
// Remove all the command definitions from the dictionary.
void Clear();
// Finds a definition for the given command.
- std::shared_ptr<const CommandDefinition> FindCommand(
- const std::string& command_name) const;
+ const CommandDefinition* FindCommand(const std::string& command_name) const;
private:
- std::shared_ptr<ObjectSchema> BuildObjectSchema(
+ using CommandMap =
+ std::map<std::string, std::unique_ptr<const CommandDefinition>>;
+
+ std::unique_ptr<ObjectSchema> BuildObjectSchema(
const base::DictionaryValue* command_def_json,
const char* property_name,
const ObjectSchema* base_def,
const std::string& command_name,
chromeos::ErrorPtr* error);
- using CommandMap = std::map<std::string,
- std::shared_ptr<const CommandDefinition>>;
-
CommandMap definitions_; // List of all available command definitions.
DISALLOW_COPY_AND_ASSIGN(CommandDictionary);
};
diff --git a/buffet/commands/command_instance.cc b/buffet/commands/command_instance.cc
index cc03210..4b13448 100644
--- a/buffet/commands/command_instance.cc
+++ b/buffet/commands/command_instance.cc
@@ -27,14 +27,13 @@
const char CommandInstance::kStatusAborted[] = "aborted";
const char CommandInstance::kStatusExpired[] = "expired";
-CommandInstance::CommandInstance(
- const std::string& name,
- const std::shared_ptr<const CommandDefinition>& command_definition,
- const native_types::Object& parameters)
- : name_(name),
- command_definition_(command_definition),
- parameters_(parameters) {
- CHECK(command_definition_.get());
+CommandInstance::CommandInstance(const std::string& name,
+ const CommandDefinition* command_definition,
+ const native_types::Object& parameters)
+ : name_{name},
+ command_definition_{command_definition},
+ parameters_{parameters} {
+ CHECK(command_definition_);
}
CommandInstance::~CommandInstance() = default;
@@ -43,13 +42,9 @@
return command_definition_->GetCategory();
}
-std::shared_ptr<const PropValue> CommandInstance::FindParameter(
- const std::string& name) const {
- std::shared_ptr<const PropValue> value;
+const PropValue* CommandInstance::FindParameter(const std::string& name) const {
auto p = parameters_.find(name);
- if (p != parameters_.end())
- value = p->second;
- return value;
+ return (p != parameters_.end()) ? p->second.get() : nullptr;
}
namespace {
@@ -87,7 +82,7 @@
// Now read in the parameters and validate their values against the command
// definition schema.
ObjectPropType obj_prop_type;
- obj_prop_type.SetObjectSchema(command_def->GetParameters());
+ obj_prop_type.SetObjectSchema(command_def->GetParameters()->Clone());
if (!TypedValueFromJson(params, &obj_prop_type, parameters, error)) {
return false;
}
@@ -130,7 +125,7 @@
}
native_types::Object parameters;
- if (!GetCommandParameters(json, command_def.get(), ¶meters, error)) {
+ if (!GetCommandParameters(json, command_def, ¶meters, error)) {
chromeos::Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain,
errors::commands::kCommandFailed,
"Failed to validate command '%s'",
diff --git a/buffet/commands/command_instance.h b/buffet/commands/command_instance.h
index 45cf485..c925b6c 100644
--- a/buffet/commands/command_instance.h
+++ b/buffet/commands/command_instance.h
@@ -32,10 +32,9 @@
// Construct a command instance given the full command |name| which must
// be in format "<package_name>.<command_name>", a command |category| and
// a list of parameters and their values specified in |parameters|.
- CommandInstance(
- const std::string& name,
- const std::shared_ptr<const CommandDefinition>& command_definition,
- const native_types::Object& parameters);
+ CommandInstance(const std::string& name,
+ const CommandDefinition* command_definition,
+ const native_types::Object& parameters);
~CommandInstance();
// Returns the full command ID.
@@ -49,11 +48,11 @@
// Returns the command results and their values.
const native_types::Object& GetResults() const { return results_; }
// Finds a command parameter value by parameter |name|. If the parameter
- // with given name does not exist, returns null shared_ptr.
- std::shared_ptr<const PropValue> FindParameter(const std::string& name) const;
+ // with given name does not exist, returns nullptr.
+ const PropValue* FindParameter(const std::string& name) const;
// Returns command definition.
- std::shared_ptr<const CommandDefinition> GetCommandDefinition() const {
+ const CommandDefinition* GetCommandDefinition() const {
return command_definition_;
}
@@ -117,7 +116,7 @@
// Full command name as "<package_name>.<command_name>".
std::string name_;
// Command definition.
- std::shared_ptr<const CommandDefinition> command_definition_;
+ const CommandDefinition* command_definition_;
// Command parameters and their values.
native_types::Object parameters_;
// Command results.
diff --git a/buffet/commands/command_instance_unittest.cc b/buffet/commands/command_instance_unittest.cc
index 420361e..98c23e5 100644
--- a/buffet/commands/command_instance_unittest.cc
+++ b/buffet/commands/command_instance_unittest.cc
@@ -88,7 +88,7 @@
EXPECT_EQ("iPityDaFool",
instance.FindParameter("phrase")->GetString()->GetValue());
EXPECT_EQ(5, instance.FindParameter("volume")->GetInt()->GetValue());
- EXPECT_EQ(nullptr, instance.FindParameter("blah").get());
+ EXPECT_EQ(nullptr, instance.FindParameter("blah"));
EXPECT_EQ(results, instance.GetResults());
}
diff --git a/buffet/commands/command_queue_unittest.cc b/buffet/commands/command_queue_unittest.cc
index 12c5500..9587797 100644
--- a/buffet/commands/command_queue_unittest.cc
+++ b/buffet/commands/command_queue_unittest.cc
@@ -17,17 +17,20 @@
namespace {
-std::unique_ptr<buffet::CommandInstance> CreateDummyCommandInstance(
- const std::string& name, const std::string& id) {
- auto command_definition = std::make_shared<const buffet::CommandDefinition>(
- "powerd",
- std::make_shared<const buffet::ObjectSchema>(),
- std::make_shared<const buffet::ObjectSchema>());
- auto cmd = std::unique_ptr<buffet::CommandInstance>(
- new buffet::CommandInstance(name, command_definition, {}));
- cmd->SetID(id);
- return cmd;
-}
+class CommandQueueTest : public testing::Test {
+ public:
+ std::unique_ptr<buffet::CommandInstance> CreateDummyCommandInstance(
+ const std::string& name, const std::string& id) {
+ std::unique_ptr<buffet::CommandInstance> cmd{
+ new buffet::CommandInstance{name, &command_definition_, {}}};
+ cmd->SetID(id);
+ return cmd;
+ }
+
+ private:
+ buffet::CommandDefinition command_definition_{
+ "powerd", buffet::ObjectSchema::Create(), buffet::ObjectSchema::Create()};
+};
// Fake implementation of CommandDispachInterface.
// Just keeps track of commands being added to and removed from the queue.
@@ -62,13 +65,13 @@
} // anonymous namespace
-TEST(CommandQueue, Empty) {
+TEST_F(CommandQueueTest, Empty) {
buffet::CommandQueue queue;
EXPECT_TRUE(queue.IsEmpty());
EXPECT_EQ(0, queue.GetCount());
}
-TEST(CommandQueue, Add) {
+TEST_F(CommandQueueTest, Add) {
buffet::CommandQueue queue;
queue.Add(CreateDummyCommandInstance("base.reboot", "id1"));
queue.Add(CreateDummyCommandInstance("base.reboot", "id2"));
@@ -77,7 +80,7 @@
EXPECT_FALSE(queue.IsEmpty());
}
-TEST(CommandQueue, Remove) {
+TEST_F(CommandQueueTest, Remove) {
buffet::CommandQueue queue;
const std::string id1 = "id1";
const std::string id2 = "id2";
@@ -97,7 +100,7 @@
EXPECT_TRUE(queue.IsEmpty());
}
-TEST(CommandQueue, Dispatch) {
+TEST_F(CommandQueueTest, Dispatch) {
FakeDispatchInterface dispatch;
buffet::CommandQueue queue;
queue.SetCommandDispachInterface(&dispatch);
@@ -115,7 +118,7 @@
EXPECT_EQ("", dispatch.GetIDs());
}
-TEST(CommandQueue, Find) {
+TEST_F(CommandQueueTest, Find) {
buffet::CommandQueue queue;
const std::string id1 = "id1";
const std::string id2 = "id2";
diff --git a/buffet/commands/dbus_command_proxy.cc b/buffet/commands/dbus_command_proxy.cc
index a067772..b300fe3 100644
--- a/buffet/commands/dbus_command_proxy.cc
+++ b/buffet/commands/dbus_command_proxy.cc
@@ -81,7 +81,7 @@
auto results_schema = command_instance_->GetCommandDefinition()->GetResults();
native_types::Object obj;
- if (!ObjectFromDBusVariant(results_schema.get(), results, &obj, error))
+ if (!ObjectFromDBusVariant(results_schema, results, &obj, error))
return false;
command_instance_->SetResults(obj);
diff --git a/buffet/commands/object_schema.cc b/buffet/commands/object_schema.cc
index b89de98..5b565ed 100644
--- a/buffet/commands/object_schema.cc
+++ b/buffet/commands/object_schema.cc
@@ -221,9 +221,22 @@
} // anonymous namespace
+ObjectSchema::ObjectSchema() {}
+ObjectSchema::~ObjectSchema() {}
+
+std::unique_ptr<ObjectSchema> ObjectSchema::Clone() const {
+ std::unique_ptr<ObjectSchema> cloned{new ObjectSchema};
+ for (const auto& pair : properties_) {
+ cloned->properties_.emplace(pair.first, pair.second->Clone());
+ }
+ cloned->extra_properties_allowed_ = extra_properties_allowed_;
+ return cloned;
+}
+
void ObjectSchema::AddProp(const std::string& name,
- std::shared_ptr<PropType> prop) {
- properties_[name] = prop;
+ std::unique_ptr<PropType> prop) {
+ // Not using emplace() here to make sure we override existing properties.
+ properties_[name] = std::move(prop);
}
const PropType* ObjectSchema::GetProp(const std::string& name) const {
@@ -254,7 +267,7 @@
object_schema ? object_schema->GetProp(iter.key()) : nullptr;
auto prop_type = PropFromJson(iter.value(), base_schema, error);
if (prop_type) {
- properties.insert(std::make_pair(iter.key(), std::move(prop_type)));
+ properties.emplace(iter.key(), std::move(prop_type));
} else {
chromeos::Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain,
errors::commands::kInvalidPropDef,
@@ -301,4 +314,8 @@
return {};
}
+std::unique_ptr<ObjectSchema> ObjectSchema::Create() {
+ return std::unique_ptr<ObjectSchema>{new ObjectSchema};
+}
+
} // namespace buffet
diff --git a/buffet/commands/object_schema.h b/buffet/commands/object_schema.h
index e47cf14..1f272e6 100644
--- a/buffet/commands/object_schema.h
+++ b/buffet/commands/object_schema.h
@@ -27,21 +27,25 @@
// these type of object description is the same.
class ObjectSchema final {
public:
+ // Do not inline the constructor/destructor to allow forward-declared type
+ // PropType to be part of |properties_| member.
+ ObjectSchema();
+ ~ObjectSchema();
+
// Properties is a string-to-PropType map representing a list of
// properties defined for a command/object. The key is the parameter
// name and the value is the parameter type definition object.
- using Properties = std::map<std::string, std::shared_ptr<PropType>>;
+ using Properties = std::map<std::string, std::unique_ptr<PropType>>;
- // Declaring default and copy constructors to document the copyable
- // nature of this class. Using the default implementation for them though.
- ObjectSchema() = default;
- ObjectSchema(const ObjectSchema& rhs) = default;
- ObjectSchema& operator=(const ObjectSchema& rhs) = default;
+ // Makes a full copy of this object.
+ virtual std::unique_ptr<ObjectSchema> Clone() const;
// Add a new parameter definition.
- void AddProp(const std::string& name, std::shared_ptr<PropType> prop);
+ void AddProp(const std::string& name, std::unique_ptr<PropType> prop);
+
// Finds parameter type definition by name. Returns nullptr if not found.
const PropType* GetProp(const std::string& name) const;
+
// Gets the list of all the properties defined.
const Properties& GetProps() const { return properties_; }
@@ -60,12 +64,16 @@
// the overridden (not inherited) ones are saved.
std::unique_ptr<base::DictionaryValue> ToJson(
bool full_schema, chromeos::ErrorPtr* error) const;
+
// Loads the object schema from JSON. If |object_schema| is not nullptr, it is
// used as a base schema to inherit omitted properties and constraints from.
bool FromJson(const base::DictionaryValue* value,
const ObjectSchema* object_schema,
chromeos::ErrorPtr* error);
+ // Helper factory method to create a new instance of ObjectSchema object.
+ static std::unique_ptr<ObjectSchema> Create();
+
// Helper method to load property type definitions from JSON.
static std::unique_ptr<PropType> PropFromJson(const base::Value& value,
const PropType* base_schema,
diff --git a/buffet/commands/object_schema_unittest.cc b/buffet/commands/object_schema_unittest.cc
index 2031705..5e913f8 100644
--- a/buffet/commands/object_schema_unittest.cc
+++ b/buffet/commands/object_schema_unittest.cc
@@ -14,6 +14,7 @@
#include <base/values.h>
#include <gtest/gtest.h>
+#include "buffet/commands/prop_constraints.h"
#include "buffet/commands/prop_types.h"
#include "buffet/commands/schema_constants.h"
#include "buffet/commands/unittest_utils.h"
@@ -516,12 +517,12 @@
EXPECT_EQ("{}", ValueToString(prop2.ToJson(false, nullptr).get()));
EXPECT_TRUE(prop2.IsBasedOnSchema());
- auto schema = std::make_shared<buffet::ObjectSchema>();
- schema->AddProp("expires", std::make_shared<buffet::IntPropType>());
- auto pw = std::make_shared<buffet::StringPropType>();
- pw->AddLengthConstraint(6, 100);
- schema->AddProp("password", pw);
- prop2.SetObjectSchema(schema);
+ auto schema = buffet::ObjectSchema::Create();
+ schema->AddProp("expires", buffet::PropType::Create(buffet::ValueType::Int));
+ auto pw = buffet::PropType::Create(buffet::ValueType::String);
+ pw->GetString()->AddLengthConstraint(6, 100);
+ schema->AddProp("password", std::move(pw));
+ prop2.SetObjectSchema(std::move(schema));
EXPECT_EQ("{'properties':{'expires':'integer',"
"'password':{'maxLength':100,'minLength':6}}}",
ValueToString(prop2.ToJson(false, nullptr).get()));
diff --git a/buffet/commands/prop_constraints.cc b/buffet/commands/prop_constraints.cc
index ff01e05..45448bc 100644
--- a/buffet/commands/prop_constraints.cc
+++ b/buffet/commands/prop_constraints.cc
@@ -99,9 +99,15 @@
return true;
}
-std::shared_ptr<Constraint>
- ConstraintStringLengthMin::CloneAsInherited() const {
- return std::make_shared<ConstraintStringLengthMin>(limit_.value);
+std::unique_ptr<Constraint>
+ConstraintStringLengthMin::Clone() const {
+ return std::unique_ptr<Constraint>{new ConstraintStringLengthMin{limit_}};
+}
+
+std::unique_ptr<Constraint>
+ConstraintStringLengthMin::CloneAsInherited() const {
+ return std::unique_ptr<Constraint>{
+ new ConstraintStringLengthMin{limit_.value}};
}
// ConstraintStringLengthMax --------------------------------------------------
@@ -126,9 +132,15 @@
return true;
}
-std::shared_ptr<Constraint>
- ConstraintStringLengthMax::CloneAsInherited() const {
- return std::make_shared<ConstraintStringLengthMax>(limit_.value);
+std::unique_ptr<Constraint>
+ConstraintStringLengthMax::Clone() const {
+ return std::unique_ptr<Constraint>{new ConstraintStringLengthMax{limit_}};
+}
+
+std::unique_ptr<Constraint>
+ConstraintStringLengthMax::CloneAsInherited() const {
+ return std::unique_ptr<Constraint>{
+ new ConstraintStringLengthMax{limit_.value}};
}
} // namespace buffet
diff --git a/buffet/commands/prop_constraints.h b/buffet/commands/prop_constraints.h
index a898e0b..79cc72a 100644
--- a/buffet/commands/prop_constraints.h
+++ b/buffet/commands/prop_constraints.h
@@ -39,24 +39,32 @@
// Gets the constraint type.
virtual ConstraintType GetType() const = 0;
+
// Checks if any of the constraint properties/attributes are overridden
// from their base schema definition. If the constraint is inherited, then
// it will not be written to JSON when saving partial schema.
virtual bool HasOverriddenAttributes() const = 0;
+
// Validates a parameter against the constraint. Returns true if parameter
// value satisfies the constraint, otherwise fills the optional |error| with
// the details for the failure.
virtual bool Validate(const PropValue& value,
chromeos::ErrorPtr* error) const = 0;
+
+ // Makes a full copy of this Constraint instance.
+ virtual std::unique_ptr<Constraint> Clone() const = 0;
+
// Makes a copy of the constraint object, marking all the attributes
// as inherited from the original definition.
- virtual std::shared_ptr<Constraint> CloneAsInherited() const = 0;
+ virtual std::unique_ptr<Constraint> CloneAsInherited() const = 0;
+
// Saves the constraint into the specified JSON |dict| object, representing
// the object schema. If |overridden_only| is set to true, then the
// inherited constraints will not be added to the schema object.
virtual bool AddToJsonDict(base::DictionaryValue* dict,
bool overridden_only,
chromeos::ErrorPtr* error) const;
+
// Saves the value of constraint to JSON value. E.g., if the numeric
// constraint was defined as {"minimum":20} this will create a JSON value
// of 20. The current design implies that each constraint has one value
@@ -64,6 +72,7 @@
// accordingly.
virtual std::unique_ptr<base::Value> ToJson(
chromeos::ErrorPtr* error) const = 0;
+
// Overloaded by the concrete class implementation, it should return the
// JSON object property name to store the constraint's value as.
// E.g., if the numeric constraint was defined as {"minimum":20} this
@@ -144,9 +153,14 @@
return true;
}
+ // Implementation of Constraint::Clone().
+ std::unique_ptr<Constraint> Clone() const override {
+ return std::unique_ptr<Constraint>{new ConstraintMin{this->limit_}};
+ }
+
// Implementation of Constraint::CloneAsInherited().
- std::shared_ptr<Constraint> CloneAsInherited() const override {
- return std::make_shared<ConstraintMin>(this->limit_.value);
+ std::unique_ptr<Constraint> CloneAsInherited() const override {
+ return std::unique_ptr<Constraint>{new ConstraintMin{this->limit_.value}};
}
// Implementation of Constraint::GetDictKey().
@@ -181,9 +195,14 @@
return true;
}
+ // Implementation of Constraint::Clone().
+ std::unique_ptr<Constraint> Clone() const override {
+ return std::unique_ptr<Constraint>{new ConstraintMax{this->limit_}};
+ }
+
// Implementation of Constraint::CloneAsInherited().
- std::shared_ptr<Constraint> CloneAsInherited() const override {
- return std::make_shared<ConstraintMax>(this->limit_.value);
+ std::unique_ptr<Constraint> CloneAsInherited() const override {
+ return std::unique_ptr<Constraint>{new ConstraintMax{this->limit_.value}};
}
// Implementation of Constraint::GetDictKey().
@@ -221,19 +240,26 @@
public:
explicit ConstraintStringLengthMin(const InheritableAttribute<int>& limit);
explicit ConstraintStringLengthMin(int limit);
+
// Implementation of Constraint::GetType().
ConstraintType GetType() const override {
return ConstraintType::StringLengthMin;
}
+
// Implementation of Constraint::Validate().
bool Validate(const PropValue& value,
chromeos::ErrorPtr* error) const override;
+
+ // Implementation of Constraint::Clone().
+ std::unique_ptr<Constraint> Clone() const override;
+
// Implementation of Constraint::CloneAsInherited().
- std::shared_ptr<Constraint> CloneAsInherited() const override;
+ std::unique_ptr<Constraint> CloneAsInherited() const override;
// Implementation of Constraint::GetDictKey().
const char* GetDictKey() const override {
return commands::attributes::kString_MinLength;
}
+
private:
DISALLOW_COPY_AND_ASSIGN(ConstraintStringLengthMin);
};
@@ -243,15 +269,22 @@
public:
explicit ConstraintStringLengthMax(const InheritableAttribute<int>& limit);
explicit ConstraintStringLengthMax(int limit);
+
// Implementation of Constraint::GetType().
ConstraintType GetType() const override {
return ConstraintType::StringLengthMax;
}
+
// Implementation of Constraint::Validate().
bool Validate(const PropValue& value,
chromeos::ErrorPtr* error) const override;
+
+ // Implementation of Constraint::Clone().
+ std::unique_ptr<Constraint> Clone() const override;
+
// Implementation of Constraint::CloneAsInherited().
- std::shared_ptr<Constraint> CloneAsInherited() const override;
+ std::unique_ptr<Constraint> CloneAsInherited() const override;
+
// Implementation of Constraint::GetDictKey().
const char* GetDictKey() const override {
return commands::attributes::kString_MaxLength;
@@ -297,9 +330,14 @@
return ReportErrorNotOneOf(error, ToString(v), values);
}
+ // Implementation of Constraint::Clone().
+ std::unique_ptr<Constraint> Clone() const override {
+ return std::unique_ptr<Constraint>{new ConstraintOneOf{set_}};
+ }
+
// Implementation of Constraint::CloneAsInherited().
- std::shared_ptr<Constraint> CloneAsInherited() const override {
- return std::make_shared<ConstraintOneOf>(set_.value);
+ std::unique_ptr<Constraint> CloneAsInherited() const override {
+ return std::unique_ptr<Constraint>{new ConstraintOneOf{set_.value}};
}
// Implementation of Constraint::ToJson().
diff --git a/buffet/commands/prop_types.cc b/buffet/commands/prop_types.cc
index abba22d..53beb4f 100644
--- a/buffet/commands/prop_types.cc
+++ b/buffet/commands/prop_types.cc
@@ -96,8 +96,12 @@
std::unique_ptr<PropType> PropType::Clone() const {
auto cloned = PropType::Create(GetType());
cloned->based_on_schema_ = based_on_schema_;
- cloned->constraints_ = constraints_;
- cloned->default_ = default_;
+ for (const auto& pair : constraints_) {
+ cloned->constraints_.emplace(pair.first, pair.second->Clone());
+ }
+ cloned->default_.is_inherited = default_.is_inherited;
+ if (default_.value)
+ cloned->default_.value = default_.value->Clone();
return cloned;
}
@@ -126,8 +130,7 @@
return false;
if (base_schema) {
for (const auto& pair : base_schema->GetConstraints()) {
- std::shared_ptr<Constraint> inherited(pair.second->CloneAsInherited());
- constraints_.insert(std::make_pair(pair.first, inherited));
+ constraints_.emplace(pair.first, pair.second->CloneAsInherited());
}
}
if (!ConstraintsFromJson(value, &processed_keys, error))
@@ -152,7 +155,7 @@
// 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::shared_ptr<PropValue> prop_value = CreateValue();
+ std::unique_ptr<PropValue> prop_value = CreateValue();
if (!prop_value->FromJson(defval, error) ||
!ValidateValue(prop_value->GetValueAsAny(), error)) {
chromeos::Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain,
@@ -161,22 +164,23 @@
commands::attributes::kDefault);
return false;
}
- default_.value = prop_value;
+ default_.value = std::move(prop_value);
default_.is_inherited = false;
} else if (base_schema) {
// If we have the base schema, inherit the type's default value from it.
// It doesn't matter if the base schema actually has a default value
// specified or not. If it doesn't, then the current type definition will
- // have no default value set either (|default_.value| is a shared_ptr to
+ // have no default value set either (|default_.value| is a unique_ptr to
// PropValue, which can be set to nullptr).
- default_.value = base_schema->default_.value;
+ if (base_schema->default_.value)
+ default_.value = base_schema->default_.value->Clone();
default_.is_inherited = true;
}
return true;
}
-void PropType::AddConstraint(const std::shared_ptr<Constraint>& constraint) {
- constraints_[constraint->GetType()] = constraint;
+void PropType::AddConstraint(std::unique_ptr<Constraint> constraint) {
+ constraints_[constraint->GetType()] = std::move(constraint);
}
void PropType::RemoveConstraint(ConstraintType constraint_type) {
@@ -200,14 +204,14 @@
bool PropType::ValidateValue(const base::Value* value,
chromeos::ErrorPtr* error) const {
- std::shared_ptr<PropValue> val = CreateValue();
+ std::unique_ptr<PropValue> val = CreateValue();
CHECK(val) << "Failed to create value object";
return val->FromJson(value, error) && ValidateConstraints(*val, error);
}
bool PropType::ValidateValue(const chromeos::Any& value,
chromeos::ErrorPtr* error) const {
- std::shared_ptr<PropValue> val = CreateValue(value, error);
+ std::unique_ptr<PropValue> val = CreateValue(value, error);
return val && ValidateConstraints(*val, error);
}
@@ -281,44 +285,48 @@
}
template<typename T>
-static std::shared_ptr<Constraint> LoadOneOfConstraint(
+static std::unique_ptr<Constraint> LoadOneOfConstraint(
const base::DictionaryValue* value,
- const std::shared_ptr<const PropType>& prop_type,
+ const PropType* prop_type,
chromeos::ErrorPtr* error) {
+ std::unique_ptr<Constraint> constraint;
const base::ListValue* list = nullptr;
if (!value->GetListWithoutPathExpansion(commands::attributes::kOneOf_Enum,
&list)) {
chromeos::Error::AddTo(error, FROM_HERE, errors::commands::kDomain,
errors::commands::kTypeMismatch,
"Expecting an array");
- return std::shared_ptr<Constraint>();
+ return constraint;
}
std::vector<T> set;
set.reserve(list->GetSize());
for (const base::Value* item : *list) {
T val{};
- if (!TypedValueFromJson(item, prop_type.get(), &val, error))
- return std::shared_ptr<Constraint>();
+ if (!TypedValueFromJson(item, prop_type, &val, error))
+ return constraint;
set.push_back(val);
}
InheritableAttribute<std::vector<T>> val(set, false);
- return std::make_shared<ConstraintOneOf<T>>(val);
+ constraint.reset(new ConstraintOneOf<T>{val});
+ return constraint;
}
template<class ConstraintClass, typename T>
-static std::shared_ptr<Constraint> LoadMinMaxConstraint(
+static std::unique_ptr<Constraint> LoadMinMaxConstraint(
const char* dict_key,
const base::DictionaryValue* value,
chromeos::ErrorPtr* error) {
+ std::unique_ptr<Constraint> constraint;
InheritableAttribute<T> limit;
const base::Value* src_val = nullptr;
CHECK(value->Get(dict_key, &src_val)) << "Unable to get min/max constraints";
if (!TypedValueFromJson(src_val, nullptr, &limit.value, error))
- return std::shared_ptr<Constraint>();
+ return constraint;
limit.is_inherited = false;
- return std::make_shared<ConstraintClass>(limit);
+ constraint.reset(new ConstraintClass{limit});
+ return constraint;
}
// PropTypeBase ----------------------------------------------------------------
@@ -334,10 +342,10 @@
if (value->HasKey(commands::attributes::kOneOf_Enum)) {
auto type = Clone();
type->RemoveAllConstraints();
- auto constraint = LoadOneOfConstraint<T>(value, std::move(type), error);
+ auto constraint = LoadOneOfConstraint<T>(value, type.get(), error);
if (!constraint)
return false;
- this->AddConstraint(constraint);
+ this->AddConstraint(std::move(constraint));
this->RemoveConstraint(ConstraintType::Min);
this->RemoveConstraint(ConstraintType::Max);
processed_keys->insert(commands::attributes::kOneOf_Enum);
@@ -365,7 +373,7 @@
commands::attributes::kNumeric_Min, value, error);
if (!constraint)
return false;
- this->AddConstraint(constraint);
+ this->AddConstraint(std::move(constraint));
this->RemoveConstraint(ConstraintType::OneOf);
processed_keys->insert(commands::attributes::kNumeric_Min);
}
@@ -374,7 +382,7 @@
commands::attributes::kNumeric_Max, value, error);
if (!constraint)
return false;
- this->AddConstraint(constraint);
+ this->AddConstraint(std::move(constraint));
this->RemoveConstraint(ConstraintType::OneOf);
processed_keys->insert(commands::attributes::kNumeric_Max);
}
@@ -401,7 +409,7 @@
commands::attributes::kString_MinLength, value, error);
if (!constraint)
return false;
- AddConstraint(constraint);
+ AddConstraint(std::move(constraint));
RemoveConstraint(ConstraintType::OneOf);
processed_keys->insert(commands::attributes::kString_MinLength);
}
@@ -410,7 +418,7 @@
commands::attributes::kString_MaxLength, value, error);
if (!constraint)
return false;
- AddConstraint(constraint);
+ AddConstraint(std::move(constraint));
RemoveConstraint(ConstraintType::OneOf);
processed_keys->insert(commands::attributes::kString_MaxLength);
}
@@ -421,8 +429,10 @@
void StringPropType::AddLengthConstraint(int min_len, int max_len) {
InheritableAttribute<int> min_attr(min_len, false);
InheritableAttribute<int> max_attr(max_len, false);
- AddConstraint(std::make_shared<ConstraintStringLengthMin>(min_attr));
- AddConstraint(std::make_shared<ConstraintStringLengthMax>(max_attr));
+ AddConstraint(std::unique_ptr<ConstraintStringLengthMin>{
+ new ConstraintStringLengthMin{min_attr}});
+ AddConstraint(std::unique_ptr<ConstraintStringLengthMax>{
+ new ConstraintStringLengthMax{max_attr}});
}
int StringPropType::GetMinLength() const {
@@ -440,7 +450,7 @@
// ObjectPropType -------------------------------------------------------------
ObjectPropType::ObjectPropType()
- : object_schema_(std::make_shared<ObjectSchema>(), false) {}
+ : object_schema_{ObjectSchema::Create(), false} {}
bool ObjectPropType::HasOverriddenAttributes() const {
return PropType::HasOverriddenAttributes() ||
@@ -449,7 +459,10 @@
std::unique_ptr<PropType> ObjectPropType::Clone() const {
auto cloned = _Base::Clone();
- cloned->GetObject()->object_schema_ = object_schema_;
+
+ cloned->GetObject()->object_schema_.is_inherited =
+ object_schema_.is_inherited;
+ cloned->GetObject()->object_schema_.value = object_schema_.value->Clone();
return cloned;
}
@@ -481,24 +494,24 @@
using commands::attributes::kObject_Properties;
- std::shared_ptr<const ObjectSchema> base_object_schema;
+ const ObjectSchema* base_object_schema = nullptr;
if (base_schema)
- base_object_schema = base_schema->GetObject()->GetObjectSchema();
+ base_object_schema = base_schema->GetObject()->GetObjectSchemaPtr();
const base::DictionaryValue* props = nullptr;
if (value->GetDictionaryWithoutPathExpansion(kObject_Properties, &props)) {
processed_keys->insert(kObject_Properties);
- auto object_schema = std::make_shared<ObjectSchema>();
- if (!object_schema->FromJson(props, base_object_schema.get(), error)) {
+ std::unique_ptr<ObjectSchema> object_schema{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;
}
- object_schema_.value = object_schema;
+ object_schema_.value = std::move(object_schema);
object_schema_.is_inherited = false;
} else if (base_object_schema) {
- object_schema_.value = base_object_schema;
+ object_schema_.value = base_object_schema->Clone();
object_schema_.is_inherited = true;
} else {
chromeos::Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain,
@@ -512,4 +525,10 @@
return true;
}
+void ObjectPropType::SetObjectSchema(
+ std::unique_ptr<const ObjectSchema> schema) {
+ object_schema_.value = std::move(schema);
+ object_schema_.is_inherited = false;
+}
+
} // namespace buffet
diff --git a/buffet/commands/prop_types.h b/buffet/commands/prop_types.h
index a2d4a18..892a730 100644
--- a/buffet/commands/prop_types.h
+++ b/buffet/commands/prop_types.h
@@ -38,7 +38,7 @@
// of each type (e.g. it makes no sense to impose two "minimum" constraints
// onto a numeric parameter).
using ConstraintMap = std::map<ConstraintType,
- std::shared_ptr<Constraint>>;
+ std::unique_ptr<Constraint>>;
PropType();
virtual ~PropType();
@@ -82,8 +82,8 @@
// Creates an instance of associated value object, using the parameter
// type as a factory class.
- virtual std::shared_ptr<PropValue> CreateValue() const = 0;
- virtual std::shared_ptr<PropValue> CreateValue(
+ virtual std::unique_ptr<PropValue> CreateValue() const = 0;
+ virtual std::unique_ptr<PropValue> CreateValue(
const chromeos::Any& val, chromeos::ErrorPtr* error) const = 0;
// Saves the parameter type definition as a JSON object.
@@ -144,7 +144,7 @@
static std::unique_ptr<PropType> Create(ValueType type);
// Adds a constraint to the type definition.
- void AddConstraint(const std::shared_ptr<Constraint>& constraint);
+ void AddConstraint(std::unique_ptr<Constraint> constraint);
// Removes a constraint of given type, if it exists.
void RemoveConstraint(ConstraintType constraint_type);
// Removes all constraints.
@@ -173,7 +173,7 @@
// value is used if the parameter value is omitted when sending a command.
// Otherwise the parameter is treated as required and, if it is omitted,
// this is treated as an error.
- InheritableAttribute<std::shared_ptr<PropValue>> default_;
+ InheritableAttribute<std::unique_ptr<PropValue>> default_;
};
// Base class for all the derived concrete implementations of property
@@ -182,14 +182,14 @@
class PropTypeBase : public PropType {
public:
ValueType GetType() const override { return GetValueType<T>(); }
- std::shared_ptr<PropValue> CreateValue() const override {
- return std::make_shared<Value>(this);
+ std::unique_ptr<PropValue> CreateValue() const override {
+ return std::unique_ptr<PropValue>{new Value{Clone()}};
}
- std::shared_ptr<PropValue> CreateValue(
+ std::unique_ptr<PropValue> CreateValue(
const chromeos::Any& v, chromeos::ErrorPtr* error) const override {
- std::shared_ptr<PropValue> prop_value;
+ std::unique_ptr<PropValue> prop_value;
if (v.IsTypeCompatible<T>()) {
- auto value = std::make_shared<Value>(this);
+ std::unique_ptr<Value> value{new Value{Clone()}};
value->SetValue(v.Get<T>());
prop_value = std::move(value);
} else {
@@ -223,8 +223,10 @@
void AddMinMaxConstraint(T min_value, T max_value) {
InheritableAttribute<T> min_attr(min_value, false);
InheritableAttribute<T> max_attr(max_value, false);
- this->AddConstraint(std::make_shared<ConstraintMin<T>>(min_attr));
- this->AddConstraint(std::make_shared<ConstraintMax<T>>(max_attr));
+ this->AddConstraint(std::unique_ptr<ConstraintMin<T>>{
+ new ConstraintMin<T>{min_attr}});
+ this->AddConstraint(std::unique_ptr<ConstraintMax<T>>{
+ new ConstraintMax<T>{max_attr}});
}
T GetMinValue() const {
auto mmc = static_cast<const ConstraintMin<T>*>(
@@ -308,18 +310,12 @@
// Returns a schema for Object-type parameter.
inline const ObjectSchema* GetObjectSchemaPtr() const {
- return GetObjectSchema().get();
+ return object_schema_.value.get();
}
- std::shared_ptr<const ObjectSchema> GetObjectSchema() const {
- return object_schema_.value;
- }
- void SetObjectSchema(const std::shared_ptr<const ObjectSchema>& schema) {
- object_schema_.value = schema;
- object_schema_.is_inherited = false;
- }
+ void SetObjectSchema(std::unique_ptr<const ObjectSchema> schema);
private:
- InheritableAttribute<std::shared_ptr<const ObjectSchema>> object_schema_;
+ InheritableAttribute<std::unique_ptr<const ObjectSchema>> object_schema_;
};
} // namespace buffet
diff --git a/buffet/commands/prop_values.cc b/buffet/commands/prop_values.cc
index 0c98a20..a4dc391 100644
--- a/buffet/commands/prop_values.cc
+++ b/buffet/commands/prop_values.cc
@@ -2,11 +2,18 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-// TODO(avakulenko) Remove this file by Aug 1, 2014 if nothing ends up here...
-
#include "buffet/commands/prop_values.h"
+#include "buffet/commands/prop_types.h"
+
namespace buffet {
+PropValue::PropValue(std::unique_ptr<const PropType> type)
+ : type_{std::move(type)} {}
+
+PropValue::PropValue(const PropType* type_ptr)
+ : type_{type_ptr->Clone()} {}
+
+PropValue::~PropValue() {}
} // namespace buffet
diff --git a/buffet/commands/prop_values.h b/buffet/commands/prop_values.h
index bf0d88d..6f85a0c 100644
--- a/buffet/commands/prop_values.h
+++ b/buffet/commands/prop_values.h
@@ -69,9 +69,13 @@
// This is used to validate the values against "enum"/"one of" constraints.
class PropValue {
public:
- explicit PropValue(const PropType* type)
- : type_(type) {}
- virtual ~PropValue() = default;
+ explicit PropValue(std::unique_ptr<const PropType> type);
+ // Special out-of-line constructor to help implement PropValue::Clone().
+ // That method needs to clone the underlying type but can't do this in this
+ // header file since PropType is just forward-declared (it needs PropValue
+ // fully defined in its own inner workings).
+ explicit PropValue(const PropType* type_ptr);
+ virtual ~PropValue();
// Gets the type of the value.
virtual ValueType GetType() const = 0;
@@ -89,7 +93,7 @@
virtual ObjectValue const* GetObject() const { return nullptr; }
// Makes a full copy of this value class.
- virtual std::shared_ptr<PropValue> Clone() const = 0;
+ virtual std::unique_ptr<PropValue> Clone() const = 0;
// Saves the value as a JSON object.
// If it fails, returns nullptr value and fills in the details for the
@@ -106,12 +110,12 @@
virtual chromeos::Any GetValueAsAny() const = 0;
// Return the type definition of this value.
- const PropType* GetPropType() const { return type_; }
+ const PropType* GetPropType() const { return type_.get(); }
// Compares two values and returns true if they are equal.
virtual bool IsEqual(const PropValue* value) const = 0;
protected:
- const PropType* type_; // weak pointer
+ std::unique_ptr<const PropType> type_;
};
// A helper template base class for implementing simple (non-Object) value
@@ -127,8 +131,11 @@
// Overrides from PropValue base class.
ValueType GetType() const override { return GetValueType<T>(); }
- std::shared_ptr<PropValue> Clone() const override {
- return std::make_shared<Derived>(*static_cast<const Derived*>(this));
+
+ std::unique_ptr<PropValue> Clone() const override {
+ std::unique_ptr<Derived> derived{new Derived{type_.get()}};
+ derived->value_ = value_;
+ return std::move(derived);
}
std::unique_ptr<base::Value> ToJson(
diff --git a/buffet/commands/schema_utils.cc b/buffet/commands/schema_utils.cc
index c9531c5..47516eb 100644
--- a/buffet/commands/schema_utils.cc
+++ b/buffet/commands/schema_utils.cc
@@ -129,7 +129,7 @@
for (const auto& pair : object_schema->GetProps()) {
const PropValue* def_value = pair.second->GetDefaultValue();
if (dict->HasKey(pair.first)) {
- std::shared_ptr<PropValue> value = pair.second->CreateValue();
+ auto value = pair.second->CreateValue();
const base::Value* param_value = nullptr;
CHECK(dict->GetWithoutPathExpansion(pair.first, ¶m_value))
<< "Unable to get parameter";
@@ -143,8 +143,7 @@
}
value_out->insert(std::make_pair(pair.first, std::move(value)));
} else if (def_value) {
- std::shared_ptr<PropValue> value = def_value->Clone();
- value_out->insert(std::make_pair(pair.first, std::move(value)));
+ value_out->insert(std::make_pair(pair.first, def_value->Clone()));
} else {
return ErrorMissingProperty(error, pair.first.c_str());
}
@@ -224,11 +223,11 @@
return dict;
}
-std::shared_ptr<const PropValue> PropValueFromDBusVariant(
+std::unique_ptr<const PropValue> PropValueFromDBusVariant(
const PropType* type,
const chromeos::Any& value,
chromeos::ErrorPtr* error) {
- std::shared_ptr<const PropValue> result;
+ std::unique_ptr<const PropValue> result;
if (type->GetType() == ValueType::Object) {
// Special case for object types.
// We expect the |value| to contain chromeos::VariantDictionary, while
@@ -280,8 +279,7 @@
}
obj->emplace_hint(obj->end(), pair.first, std::move(prop_value));
} else if (def_value) {
- std::shared_ptr<const PropValue> prop_value = def_value->Clone();
- obj->emplace_hint(obj->end(), pair.first, std::move(prop_value));
+ obj->emplace_hint(obj->end(), pair.first, def_value->Clone());
} else {
ErrorMissingProperty(error, pair.first.c_str());
return false;
diff --git a/buffet/commands/schema_utils.h b/buffet/commands/schema_utils.h
index 55c07a1..5709ebc 100644
--- a/buffet/commands/schema_utils.h
+++ b/buffet/commands/schema_utils.h
@@ -135,7 +135,7 @@
// Converts D-Bus variant to PropValue.
// Has special handling for Object types where chromeos::VariantDictionary
// is converted to native_types::Object.
-std::shared_ptr<const PropValue> PropValueFromDBusVariant(
+std::unique_ptr<const PropValue> PropValueFromDBusVariant(
const PropType* type,
const chromeos::Any& value,
chromeos::ErrorPtr* error);
diff --git a/buffet/commands/schema_utils_unittest.cc b/buffet/commands/schema_utils_unittest.cc
index 3cd0b5d..4f9eabe 100644
--- a/buffet/commands/schema_utils_unittest.cc
+++ b/buffet/commands/schema_utils_unittest.cc
@@ -159,25 +159,25 @@
TEST(CommandSchemaUtils, TypedValueFromJson_Object) {
buffet::native_types::Object value;
- auto schema = std::make_shared<buffet::ObjectSchema>();
+ std::unique_ptr<buffet::ObjectSchema> schema{new buffet::ObjectSchema};
- auto age_prop = std::make_shared<buffet::IntPropType>();
- age_prop->AddMinMaxConstraint(0, 150);
- schema->AddProp("age", age_prop);
+ buffet::IntPropType age_prop;
+ age_prop.AddMinMaxConstraint(0, 150);
+ schema->AddProp("age", age_prop.Clone());
- auto name_prop = std::make_shared<buffet::StringPropType>();
- name_prop->AddLengthConstraint(1, 30);
- schema->AddProp("name", name_prop);
+ buffet::StringPropType name_prop;
+ name_prop.AddLengthConstraint(1, 30);
+ schema->AddProp("name", name_prop.Clone());
buffet::ObjectPropType type;
- type.SetObjectSchema(schema);
+ type.SetObjectSchema(std::move(schema));
EXPECT_TRUE(buffet::TypedValueFromJson(
CreateValue("{'age':20,'name':'Bob'}").get(), &type, &value, nullptr));
buffet::native_types::Object value2;
- value2.insert(std::make_pair("age", age_prop->CreateValue(20, nullptr)));
+ value2.insert(std::make_pair("age", age_prop.CreateValue(20, nullptr)));
value2.insert(std::make_pair("name",
- name_prop->CreateValue(std::string("Bob"),
- nullptr)));
+ name_prop.CreateValue(std::string("Bob"),
+ nullptr)));
EXPECT_EQ(value2, value);
chromeos::ErrorPtr error;
diff --git a/buffet/commands/unittest_utils.h b/buffet/commands/unittest_utils.h
index 577405a..25db6b2 100644
--- a/buffet/commands/unittest_utils.h
+++ b/buffet/commands/unittest_utils.h
@@ -28,32 +28,29 @@
// apostrophes for easy comparisons in C++ source code.
std::string ValueToString(const base::Value* value);
-template <typename PV, typename T> std::shared_ptr<const PV>
-make_prop_value(const PropType* type, const T& value) {
- auto result = std::make_shared<PV>(type);
+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>())}};
result->SetValue(value);
- return result;
+ return std::move(result);
}
-inline std::shared_ptr<const IntValue> make_int_prop_value(int value) {
- static const PropType* int_prop_type = new IntPropType();
- return make_prop_value<IntValue, int>(int_prop_type, value);
+inline std::unique_ptr<const IntValue> make_int_prop_value(int value) {
+ return make_prop_value<IntValue, int>(value);
}
-inline std::shared_ptr<const DoubleValue> make_double_prop_value(double value) {
- static const PropType* double_prop_type = new DoublePropType();
- return make_prop_value<DoubleValue, double>(double_prop_type, value);
+inline std::unique_ptr<const DoubleValue> make_double_prop_value(double value) {
+ return make_prop_value<DoubleValue, double>(value);
}
-inline std::shared_ptr<const BooleanValue> make_bool_prop_value(bool value) {
- static const PropType* boolean_prop_type = new BooleanPropType();
- return make_prop_value<BooleanValue, bool>(boolean_prop_type, value);
+inline std::unique_ptr<const BooleanValue> make_bool_prop_value(bool value) {
+ return make_prop_value<BooleanValue, bool>(value);
}
-inline std::shared_ptr<const StringValue>
+inline std::unique_ptr<const StringValue>
make_string_prop_value(const std::string& value) {
- static const PropType* string_prop_type = new StringPropType();
- return make_prop_value<StringValue, std::string>(string_prop_type, value);
+ return make_prop_value<StringValue, std::string>(value);
}
} // namespace unittests
diff --git a/buffet/states/state_change_queue_interface.h b/buffet/states/state_change_queue_interface.h
index 0532788..44e0f79 100644
--- a/buffet/states/state_change_queue_interface.h
+++ b/buffet/states/state_change_queue_interface.h
@@ -19,9 +19,8 @@
// |changed_properties| contains a property set with the new property values
// which were updated at the time the event was recorded.
struct StateChange {
- StateChange(base::Time time,
- native_types::Object properties)
- : timestamp(time), changed_properties(std::move(properties)) {}
+ StateChange(base::Time time, native_types::Object properties)
+ : timestamp{time}, changed_properties{std::move(properties)} {}
base::Time timestamp;
native_types::Object changed_properties;
};
diff --git a/buffet/states/state_package.cc b/buffet/states/state_package.cc
index 8893ebb..a023c14 100644
--- a/buffet/states/state_package.cc
+++ b/buffet/states/state_package.cc
@@ -37,9 +37,13 @@
// Now move all the properties to |types_| object.
for (const auto& pair : schema.GetProps()) {
- types_.AddProp(pair.first, pair.second);
+ types_.AddProp(pair.first, pair.second->Clone());
// Create default value for this state property.
- values_.insert(std::make_pair(pair.first, pair.second->CreateValue()));
+ if (pair.second->GetDefaultValue()) {
+ values_.emplace(pair.first, pair.second->GetDefaultValue()->Clone());
+ } else {
+ values_.emplace(pair.first, pair.second->CreateValue());
+ }
}
return true;
@@ -61,7 +65,7 @@
auto new_value = it->second->GetPropType()->CreateValue();
if (!new_value->FromJson(&iter.value(), error))
return false;
- it->second = new_value;
+ it->second = std::move(new_value);
iter.Advance();
}
return true;
@@ -109,7 +113,7 @@
value, error);
if (!new_value)
return false;
- it->second = new_value;
+ it->second = std::move(new_value);
return true;
}
