Remove object schema parsing in state definition
State definition schema is no longer parsed by libweave for semantic
understanding. We just keep it around for clients who explicitly
as for it, but we no longer use the schema to validate the state
values. They are now stored as an opaque JSON object.
BUG: 25841230
Change-Id: Ib54b3d1f52dd8c63e9fa74afc174adeb5ca56d4d
Reviewed-on: https://weave-review.googlesource.com/1627
Reviewed-by: Vitaly Buka <vitalybuka@google.com>
diff --git a/src/states/mock_state_change_queue_interface.h b/src/states/mock_state_change_queue_interface.h
index c3f50c0..78c6c82 100644
--- a/src/states/mock_state_change_queue_interface.h
+++ b/src/states/mock_state_change_queue_interface.h
@@ -16,9 +16,11 @@
class MockStateChangeQueueInterface : public StateChangeQueueInterface {
public:
MOCK_CONST_METHOD0(IsEmpty, bool());
- MOCK_METHOD2(NotifyPropertiesUpdated,
- bool(base::Time timestamp, ValueMap changed_properties));
- MOCK_METHOD0(GetAndClearRecordedStateChanges, std::vector<StateChange>());
+ MOCK_METHOD2(MockNotifyPropertiesUpdated,
+ bool(base::Time timestamp,
+ const base::DictionaryValue& changed_properties));
+ MOCK_METHOD0(MockGetAndClearRecordedStateChanges,
+ std::vector<StateChange>&());
MOCK_CONST_METHOD0(GetLastStateChangeId, UpdateID());
MOCK_METHOD1(MockAddOnStateUpdatedCallback,
base::CallbackList<void(UpdateID)>::Subscription*(
@@ -26,6 +28,15 @@
MOCK_METHOD1(NotifyStateUpdatedOnServer, void(UpdateID));
private:
+ bool NotifyPropertiesUpdated(
+ base::Time timestamp,
+ std::unique_ptr<base::DictionaryValue> changed_properties) override {
+ return MockNotifyPropertiesUpdated(timestamp, *changed_properties);
+ }
+ std::vector<StateChange> GetAndClearRecordedStateChanges() override {
+ return std::move(MockGetAndClearRecordedStateChanges());
+ }
+
Token AddOnStateUpdatedCallback(
const base::Callback<void(UpdateID)>& callback) override {
return Token{MockAddOnStateUpdatedCallback(callback)};
diff --git a/src/states/state_change_queue.cc b/src/states/state_change_queue.cc
index 288dadc..87cc18d 100644
--- a/src/states/state_change_queue.cc
+++ b/src/states/state_change_queue.cc
@@ -13,12 +13,15 @@
CHECK_GT(max_queue_size_, 0U) << "Max queue size must not be zero";
}
-bool StateChangeQueue::NotifyPropertiesUpdated(base::Time timestamp,
- ValueMap changed_properties) {
+bool StateChangeQueue::NotifyPropertiesUpdated(
+ base::Time timestamp,
+ std::unique_ptr<base::DictionaryValue> changed_properties) {
auto& stored_changes = state_changes_[timestamp];
// Merge the old property set.
- changed_properties.insert(stored_changes.begin(), stored_changes.end());
- stored_changes = std::move(changed_properties);
+ if (stored_changes)
+ stored_changes->MergeDictionary(changed_properties.get());
+ else
+ stored_changes = std::move(changed_properties);
while (state_changes_.size() > max_queue_size_) {
// Queue is full.
@@ -30,8 +33,8 @@
auto element_old = state_changes_.begin();
auto element_new = std::next(element_old);
// This will skip elements that exist in both [old] and [new].
- element_new->second.insert(element_old->second.begin(),
- element_old->second.end());
+ element_old->second->MergeDictionary(element_new->second.get());
+ std::swap(element_old->second, element_new->second);
state_changes_.erase(element_old);
}
++last_change_id_;
@@ -41,8 +44,8 @@
std::vector<StateChange> StateChangeQueue::GetAndClearRecordedStateChanges() {
std::vector<StateChange> changes;
changes.reserve(state_changes_.size());
- for (const auto& pair : state_changes_) {
- changes.emplace_back(pair.first, std::move(pair.second));
+ for (auto& pair : state_changes_) {
+ changes.push_back(StateChange{pair.first, std::move(pair.second)});
}
state_changes_.clear();
return changes;
diff --git a/src/states/state_change_queue.h b/src/states/state_change_queue.h
index 00b827f..314f746 100644
--- a/src/states/state_change_queue.h
+++ b/src/states/state_change_queue.h
@@ -21,8 +21,9 @@
// Overrides from StateChangeQueueInterface.
bool IsEmpty() const override { return state_changes_.empty(); }
- bool NotifyPropertiesUpdated(base::Time timestamp,
- ValueMap changed_properties) override;
+ bool NotifyPropertiesUpdated(
+ base::Time timestamp,
+ std::unique_ptr<base::DictionaryValue> changed_properties) override;
std::vector<StateChange> GetAndClearRecordedStateChanges() override;
UpdateID GetLastStateChangeId() const override { return last_change_id_; }
Token AddOnStateUpdatedCallback(
@@ -35,7 +36,7 @@
const size_t max_queue_size_;
// Accumulated list of device state change notifications.
- std::map<base::Time, ValueMap> state_changes_;
+ std::map<base::Time, std::unique_ptr<base::DictionaryValue>> state_changes_;
// An ID of last state change update. Each NotifyPropertiesUpdated()
// invocation increments this value by 1.
diff --git a/src/states/state_change_queue_interface.h b/src/states/state_change_queue_interface.h
index e3b3650..81a2cfc 100644
--- a/src/states/state_change_queue_interface.h
+++ b/src/states/state_change_queue_interface.h
@@ -19,10 +19,11 @@
// |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, ValueMap properties)
+ StateChange(base::Time time,
+ std::unique_ptr<base::DictionaryValue> properties)
: timestamp{time}, changed_properties{std::move(properties)} {}
base::Time timestamp;
- ValueMap changed_properties;
+ std::unique_ptr<base::DictionaryValue> changed_properties;
};
// An abstract interface to StateChangeQueue to record and retrieve state
@@ -37,8 +38,9 @@
virtual bool IsEmpty() const = 0;
// Called by StateManager when device state properties are updated.
- virtual bool NotifyPropertiesUpdated(base::Time timestamp,
- ValueMap changed_properties) = 0;
+ virtual bool NotifyPropertiesUpdated(
+ base::Time timestamp,
+ std::unique_ptr<base::DictionaryValue> changed_properties) = 0;
// Returns the recorded state changes since last time this method was called.
virtual std::vector<StateChange> GetAndClearRecordedStateChanges() = 0;
diff --git a/src/states/state_change_queue_unittest.cc b/src/states/state_change_queue_unittest.cc
index 9f26071..5fbc012 100644
--- a/src/states/state_change_queue_unittest.cc
+++ b/src/states/state_change_queue_unittest.cc
@@ -11,6 +11,8 @@
namespace weave {
+using test::CreateDictionaryValue;
+
class StateChangeQueueTest : public ::testing::Test {
public:
void SetUp() override { queue_.reset(new StateChangeQueue(100)); }
@@ -27,43 +29,39 @@
}
TEST_F(StateChangeQueueTest, UpdateOne) {
- StateChange change{base::Time::Now(),
- ValueMap{{"prop.name", test::make_int_prop_value(23)}}};
- ASSERT_TRUE(queue_->NotifyPropertiesUpdated(change.timestamp,
- change.changed_properties));
+ auto timestamp = base::Time::Now();
+ ASSERT_TRUE(queue_->NotifyPropertiesUpdated(
+ timestamp, CreateDictionaryValue("{'prop': {'name': 23}}")));
EXPECT_FALSE(queue_->IsEmpty());
EXPECT_EQ(1, queue_->GetLastStateChangeId());
auto changes = queue_->GetAndClearRecordedStateChanges();
EXPECT_EQ(1, queue_->GetLastStateChangeId());
ASSERT_EQ(1, changes.size());
- EXPECT_EQ(change.timestamp, changes.front().timestamp);
- EXPECT_EQ(change.changed_properties, changes.front().changed_properties);
+ EXPECT_EQ(timestamp, changes.front().timestamp);
+ EXPECT_JSON_EQ("{'prop':{'name': 23}}", *changes.front().changed_properties);
EXPECT_TRUE(queue_->IsEmpty());
EXPECT_TRUE(queue_->GetAndClearRecordedStateChanges().empty());
}
-// TODO(vitalybuka): Fix flakiness.
-TEST_F(StateChangeQueueTest, DISABLED_UpdateMany) {
- StateChange change1{base::Time::Now(),
- ValueMap{{"prop.name1", test::make_int_prop_value(23)}}};
- ASSERT_TRUE(queue_->NotifyPropertiesUpdated(change1.timestamp,
- change1.changed_properties));
- StateChange change2{base::Time::Now(),
- ValueMap{
- {"prop.name1", test::make_int_prop_value(17)},
- {"prop.name2", test::make_double_prop_value(1.0)},
- {"prop.name3", test::make_bool_prop_value(false)},
- }};
- ASSERT_TRUE(queue_->NotifyPropertiesUpdated(change2.timestamp,
- change2.changed_properties));
+TEST_F(StateChangeQueueTest, UpdateMany) {
+ auto timestamp1 = base::Time::Now();
+ const std::string state1 = "{'prop': {'name1': 23}}";
+ auto timestamp2 = timestamp1 + base::TimeDelta::FromSeconds(1);
+ const std::string state2 =
+ "{'prop': {'name1': 17, 'name2': 1.0, 'name3': false}}";
+ ASSERT_TRUE(queue_->NotifyPropertiesUpdated(
+ timestamp1, CreateDictionaryValue(state1)));
+ ASSERT_TRUE(queue_->NotifyPropertiesUpdated(
+ timestamp2, CreateDictionaryValue(state2)));
+
EXPECT_EQ(2, queue_->GetLastStateChangeId());
EXPECT_FALSE(queue_->IsEmpty());
auto changes = queue_->GetAndClearRecordedStateChanges();
ASSERT_EQ(2, changes.size());
- EXPECT_EQ(change1.timestamp, changes[0].timestamp);
- EXPECT_EQ(change1.changed_properties, changes[0].changed_properties);
- EXPECT_EQ(change2.timestamp, changes[1].timestamp);
- EXPECT_EQ(change2.changed_properties, changes[1].changed_properties);
+ EXPECT_EQ(timestamp1, changes[0].timestamp);
+ EXPECT_JSON_EQ(state1, *changes[0].changed_properties);
+ EXPECT_EQ(timestamp2, changes[1].timestamp);
+ EXPECT_JSON_EQ(state2, *changes[1].changed_properties);
EXPECT_TRUE(queue_->IsEmpty());
EXPECT_TRUE(queue_->GetAndClearRecordedStateChanges().empty());
}
@@ -73,33 +71,27 @@
base::TimeDelta time_delta = base::TimeDelta::FromMinutes(1);
ASSERT_TRUE(queue_->NotifyPropertiesUpdated(
- timestamp, ValueMap{{"prop.name1", test::make_int_prop_value(1)}}));
+ timestamp, CreateDictionaryValue("{'prop': {'name1': 1}}")));
ASSERT_TRUE(queue_->NotifyPropertiesUpdated(
- timestamp, ValueMap{{"prop.name2", test::make_int_prop_value(2)}}));
+ timestamp, CreateDictionaryValue("{'prop': {'name2': 2}}")));
ASSERT_TRUE(queue_->NotifyPropertiesUpdated(
- timestamp, ValueMap{{"prop.name1", test::make_int_prop_value(3)}}));
+ timestamp, CreateDictionaryValue("{'prop': {'name1': 3}}")));
ASSERT_TRUE(queue_->NotifyPropertiesUpdated(
- timestamp + time_delta,
- ValueMap{{"prop.name1", test::make_int_prop_value(4)}}));
+ timestamp + time_delta, CreateDictionaryValue("{'prop': {'name1': 4}}")));
auto changes = queue_->GetAndClearRecordedStateChanges();
EXPECT_EQ(4, queue_->GetLastStateChangeId());
ASSERT_EQ(2, changes.size());
- ValueMap expected1{
- {"prop.name1", test::make_int_prop_value(3)},
- {"prop.name2", test::make_int_prop_value(2)},
- };
- ValueMap expected2{
- {"prop.name1", test::make_int_prop_value(4)},
- };
+ const std::string expected1 = "{'prop': {'name1': 3, 'name2': 2}}";
+ const std::string expected2 = "{'prop': {'name1': 4}}";
EXPECT_EQ(timestamp, changes[0].timestamp);
- EXPECT_EQ(expected1, changes[0].changed_properties);
+ EXPECT_JSON_EQ(expected1, *changes[0].changed_properties);
EXPECT_EQ(timestamp + time_delta, changes[1].timestamp);
- EXPECT_EQ(expected2, changes[1].changed_properties);
+ EXPECT_JSON_EQ(expected2, *changes[1].changed_properties);
}
TEST_F(StateChangeQueueTest, MaxQueueSize) {
@@ -109,43 +101,29 @@
base::TimeDelta time_delta2 = base::TimeDelta::FromMinutes(3);
ASSERT_TRUE(queue_->NotifyPropertiesUpdated(
- start_time, ValueMap{
- {"prop.name1", test::make_int_prop_value(1)},
- {"prop.name2", test::make_int_prop_value(2)},
- }));
+ start_time, CreateDictionaryValue("{'prop': {'name1': 1, 'name2': 2}}")));
ASSERT_TRUE(queue_->NotifyPropertiesUpdated(
start_time + time_delta1,
- ValueMap{
- {"prop.name1", test::make_int_prop_value(3)},
- {"prop.name3", test::make_int_prop_value(4)},
- }));
+ CreateDictionaryValue("{'prop': {'name1': 3, 'name3': 4}}")));
ASSERT_TRUE(queue_->NotifyPropertiesUpdated(
start_time + time_delta2,
- ValueMap{
- {"prop.name10", test::make_int_prop_value(10)},
- {"prop.name11", test::make_int_prop_value(11)},
- }));
+ CreateDictionaryValue("{'prop': {'name10': 10, 'name11': 11}}")));
EXPECT_EQ(3, queue_->GetLastStateChangeId());
auto changes = queue_->GetAndClearRecordedStateChanges();
ASSERT_EQ(2, changes.size());
- ValueMap expected1{
- {"prop.name1", test::make_int_prop_value(3)},
- {"prop.name2", test::make_int_prop_value(2)},
- {"prop.name3", test::make_int_prop_value(4)},
- };
+ const std::string expected1 =
+ "{'prop': {'name1': 3, 'name2': 2, 'name3': 4}}";
EXPECT_EQ(start_time + time_delta1, changes[0].timestamp);
- EXPECT_EQ(expected1, changes[0].changed_properties);
+ EXPECT_JSON_EQ(expected1, *changes[0].changed_properties);
- ValueMap expected2{
- {"prop.name10", test::make_int_prop_value(10)},
- {"prop.name11", test::make_int_prop_value(11)},
- };
+ const std::string expected2 =
+ "{'prop': {'name10': 10, 'name11': 11}}";
EXPECT_EQ(start_time + time_delta2, changes[1].timestamp);
- EXPECT_EQ(expected2, changes[1].changed_properties);
+ EXPECT_JSON_EQ(expected2, *changes[1].changed_properties);
}
TEST_F(StateChangeQueueTest, ImmediateStateChangeNotification) {
@@ -161,10 +139,8 @@
TEST_F(StateChangeQueueTest, DelayedStateChangeNotification) {
// When queue is not empty, registering a new callback will not trigger it.
ASSERT_TRUE(queue_->NotifyPropertiesUpdated(
- base::Time::Now(), ValueMap{
- {"prop.name1", test::make_int_prop_value(1)},
- {"prop.name2", test::make_int_prop_value(2)},
- }));
+ base::Time::Now(),
+ CreateDictionaryValue("{'prop': {'name1': 1, 'name3': 2}}")));
auto callback = [](StateChangeQueueInterface::UpdateID id) {
FAIL() << "This should not be called";
diff --git a/src/states/state_manager.cc b/src/states/state_manager.cc
index fe6e669..a424588 100644
--- a/src/states/state_manager.cc
+++ b/src/states/state_manager.cc
@@ -93,8 +93,9 @@
if (!package->SetPropertyValue(property_name, value, error))
return false;
- ValueMap prop_set{{full_property_name, package->GetProperty(property_name)}};
- state_change_queue_->NotifyPropertiesUpdated(timestamp, prop_set);
+ std::unique_ptr<base::DictionaryValue> prop_set{new base::DictionaryValue};
+ prop_set->Set(full_property_name, value.DeepCopy());
+ state_change_queue_->NotifyPropertiesUpdated(timestamp, std::move(prop_set));
return true;
}
diff --git a/src/states/state_manager_unittest.cc b/src/states/state_manager_unittest.cc
index 3f854fe..933e11b 100644
--- a/src/states/state_manager_unittest.cc
+++ b/src/states/state_manager_unittest.cc
@@ -22,17 +22,18 @@
using testing::_;
using testing::Return;
+using testing::ReturnRef;
using test::CreateDictionaryValue;
namespace {
const char kBaseDefinition[] = R"({
"base": {
- "manufacturer":"string",
- "serialNumber":"string"
+ "manufacturer":{"type":"string"},
+ "serialNumber":{"type":"string"}
},
"device": {
- "state_property":"string"
+ "state_property":{"type":"string"}
}
})";
@@ -51,6 +52,10 @@
return CreateDictionaryValue(kBaseDefaults);
}
+MATCHER_P(IsState, str, "") {
+ return arg.Equals(CreateDictionaryValue(str).get());
+}
+
} // anonymous namespace
class StateManagerTest : public ::testing::Test {
@@ -58,9 +63,9 @@
void SetUp() override {
// Initial expectations.
EXPECT_CALL(mock_state_change_queue_, IsEmpty()).Times(0);
- EXPECT_CALL(mock_state_change_queue_, NotifyPropertiesUpdated(_, _))
+ EXPECT_CALL(mock_state_change_queue_, MockNotifyPropertiesUpdated(_, _))
.WillRepeatedly(Return(true));
- EXPECT_CALL(mock_state_change_queue_, GetAndClearRecordedStateChanges())
+ EXPECT_CALL(mock_state_change_queue_, MockGetAndClearRecordedStateChanges())
.Times(0);
mgr_.reset(new StateManager(&mock_state_change_queue_));
@@ -102,9 +107,7 @@
'manufacturer': 'Test Factory',
'serialNumber': 'Test Model'
},
- 'device': {
- 'state_property': ''
- }
+ 'device': {}
})";
EXPECT_JSON_EQ(expected, *mgr_->GetState());
}
@@ -122,12 +125,8 @@
'manufacturer': 'Test Factory',
'serialNumber': 'Test Model'
},
- 'power': {
- 'battery_level': 0
- },
- 'device': {
- 'state_property': ''
- }
+ 'power': {},
+ 'device': {}
})";
EXPECT_JSON_EQ(expected, *mgr_->GetState());
}
@@ -137,12 +136,15 @@
auto state_definition = R"({
"base": {
- "firmwareVersion": "string",
- "localDiscoveryEnabled": "boolean",
- "localAnonymousAccessMaxRole": [ "none", "viewer", "user" ],
- "localPairingEnabled": "boolean"
+ "firmwareVersion": {"type":"string"},
+ "localDiscoveryEnabled": {"type":"boolean"},
+ "localAnonymousAccessMaxRole": {
+ "type": "string",
+ "enum": ["none", "viewer", "user"]
+ },
+ "localPairingEnabled": {"type":"boolean"}
},
- "power": {"battery_level":"integer"}
+ "power": {"battery_level":{"type":"integer"}}
})";
ASSERT_TRUE(manager.LoadStateDefinitionFromJson(state_definition, nullptr));
@@ -172,11 +174,9 @@
}
TEST_F(StateManagerTest, SetPropertyValue) {
- ValueMap expected_prop_set{
- {"device.state_property", test::make_string_prop_value("Test Value")},
- };
+ const std::string state = "{'device': {'state_property': 'Test Value'}}";
EXPECT_CALL(mock_state_change_queue_,
- NotifyPropertiesUpdated(timestamp_, expected_prop_set))
+ MockNotifyPropertiesUpdated(timestamp_, IsState(state)))
.WillOnce(Return(true));
ASSERT_TRUE(SetPropertyValue("device.state_property",
base::StringValue{"Test Value"}, nullptr));
@@ -216,39 +216,36 @@
}
TEST_F(StateManagerTest, SetPropertyValue_Error_UnknownProperty) {
- ErrorPtr error;
- ASSERT_FALSE(
- SetPropertyValue("base.level", base::FundamentalValue{0}, &error));
- EXPECT_EQ(errors::state::kDomain, error->GetDomain());
- EXPECT_EQ(errors::state::kPropertyNotDefined, error->GetCode());
+ ASSERT_TRUE(
+ SetPropertyValue("base.level", base::FundamentalValue{0}, nullptr));
}
TEST_F(StateManagerTest, GetAndClearRecordedStateChanges) {
- EXPECT_CALL(mock_state_change_queue_, NotifyPropertiesUpdated(timestamp_, _))
+ EXPECT_CALL(mock_state_change_queue_,
+ MockNotifyPropertiesUpdated(timestamp_, _))
.WillOnce(Return(true));
ASSERT_TRUE(SetPropertyValue("device.state_property",
base::StringValue{"Test Value"}, nullptr));
- std::vector<StateChange> expected_val;
- expected_val.emplace_back(
- timestamp_, ValueMap{{"device.state_property",
- test::make_string_prop_value("Test Value")}});
- EXPECT_CALL(mock_state_change_queue_, GetAndClearRecordedStateChanges())
- .WillOnce(Return(expected_val));
+ std::vector<StateChange> expected_state;
+ const std::string expected_val =
+ "{'device': {'state_property': 'Test Value'}}";
+ expected_state.emplace_back(
+ timestamp_,
+ CreateDictionaryValue(expected_val));
+ EXPECT_CALL(mock_state_change_queue_, MockGetAndClearRecordedStateChanges())
+ .WillOnce(ReturnRef(expected_state));
EXPECT_CALL(mock_state_change_queue_, GetLastStateChangeId())
.WillOnce(Return(0));
auto changes = mgr_->GetAndClearRecordedStateChanges();
ASSERT_EQ(1, changes.second.size());
- EXPECT_EQ(expected_val.back().timestamp, changes.second.back().timestamp);
- EXPECT_EQ(expected_val.back().changed_properties,
- changes.second.back().changed_properties);
+ EXPECT_EQ(timestamp_, changes.second.back().timestamp);
+ EXPECT_JSON_EQ(expected_val, *changes.second.back().changed_properties);
}
TEST_F(StateManagerTest, SetProperties) {
- ValueMap expected_prop_set{
- {"base.manufacturer", test::make_string_prop_value("No Name")},
- };
+ const std::string state = "{'base': {'manufacturer': 'No Name'}}";
EXPECT_CALL(mock_state_change_queue_,
- NotifyPropertiesUpdated(_, expected_prop_set))
+ MockNotifyPropertiesUpdated(_, IsState(state)))
.WillOnce(Return(true));
EXPECT_CALL(*this, OnStateChanged()).Times(1);
@@ -260,16 +257,14 @@
'manufacturer': 'No Name',
'serialNumber': 'Test Model'
},
- 'device': {
- 'state_property': ''
- }
+ 'device': {}
})";
EXPECT_JSON_EQ(expected, *mgr_->GetState());
}
TEST_F(StateManagerTest, GetProperty) {
EXPECT_JSON_EQ("'Test Model'", *mgr_->GetProperty("base.serialNumber"));
- EXPECT_JSON_EQ("''", *mgr_->GetProperty("device.state_property"));
+ EXPECT_EQ(nullptr, mgr_->GetProperty("device.state_property"));
EXPECT_EQ(nullptr, mgr_->GetProperty("device.unknown"));
EXPECT_EQ(nullptr, mgr_->GetProperty("unknown.state_property"));
}
diff --git a/src/states/state_package.cc b/src/states/state_package.cc
index 0b8e219..55650b6 100644
--- a/src/states/state_package.cc
+++ b/src/states/state_package.cc
@@ -7,9 +7,6 @@
#include <base/logging.h>
#include <base/values.h>
-#include "src/commands/prop_types.h"
-#include "src/commands/prop_values.h"
-#include "src/commands/schema_utils.h"
#include "src/states/error_codes.h"
namespace weave {
@@ -18,28 +15,18 @@
bool StatePackage::AddSchemaFromJson(const base::DictionaryValue* json,
ErrorPtr* error) {
- ObjectSchema schema;
- if (!schema.FromJson(json, nullptr, error))
- return false;
-
// Scan first to make sure we have no property redefinitions.
- for (const auto& pair : schema.GetProps()) {
- if (types_.GetProp(pair.first)) {
+ for (base::DictionaryValue::Iterator it(*json); !it.IsAtEnd(); it.Advance()) {
+ if (types_.HasKey(it.key())) {
Error::AddToPrintf(error, FROM_HERE, errors::state::kDomain,
errors::state::kPropertyRedefinition,
"State property '%s.%s' is already defined",
- name_.c_str(), pair.first.c_str());
+ name_.c_str(), it.key().c_str());
return false;
}
}
- // Now move all the properties to |types_| object.
- 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->CreateDefaultValue());
- }
-
+ types_.MergeDictionary(json);
return true;
}
@@ -53,20 +40,14 @@
}
std::unique_ptr<base::DictionaryValue> StatePackage::GetValuesAsJson() const {
- std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue);
- for (const auto& pair : values_) {
- auto value = pair.second->ToJson();
- CHECK(value);
- dict->SetWithoutPathExpansion(pair.first, value.release());
- }
- return dict;
+ return std::unique_ptr<base::DictionaryValue>(values_.DeepCopy());
}
std::unique_ptr<base::Value> StatePackage::GetPropertyValue(
const std::string& property_name,
ErrorPtr* error) const {
- auto it = values_.find(property_name);
- if (it == values_.end()) {
+ const base::Value* value = nullptr;
+ if (!values_.Get(property_name, &value)) {
Error::AddToPrintf(error, FROM_HERE, errors::state::kDomain,
errors::state::kPropertyNotDefined,
"State property '%s.%s' is not defined", name_.c_str(),
@@ -74,24 +55,13 @@
return nullptr;
}
- return it->second->ToJson();
+ return std::unique_ptr<base::Value>(value->DeepCopy());
}
bool StatePackage::SetPropertyValue(const std::string& property_name,
const base::Value& value,
ErrorPtr* error) {
- auto it = values_.find(property_name);
- if (it == values_.end()) {
- Error::AddToPrintf(error, FROM_HERE, errors::state::kDomain,
- errors::state::kPropertyNotDefined,
- "State property '%s.%s' is not defined", name_.c_str(),
- property_name.c_str());
- return false;
- }
- auto new_value = it->second->GetPropType()->CreatePropValue(value, error);
- if (!new_value)
- return false;
- it->second = std::move(new_value);
+ values_.Set(property_name, value.DeepCopy());
return true;
}
diff --git a/src/states/state_package.h b/src/states/state_package.h
index e8e3964..7632145 100644
--- a/src/states/state_package.h
+++ b/src/states/state_package.h
@@ -10,15 +10,9 @@
#include <string>
#include <base/macros.h>
+#include <base/values.h>
#include <weave/error.h>
-#include "src/commands/object_schema.h"
-#include "src/commands/prop_values.h"
-
-namespace base {
-class DictionaryValue;
-} // namespace base
-
namespace weave {
// A package is a set of related state properties. GCD specification defines
@@ -62,20 +56,13 @@
const base::Value& value,
ErrorPtr* error);
- std::shared_ptr<const PropValue> GetProperty(
- const std::string& property_name) const {
- auto it = values_.find(property_name);
- return it != values_.end() ? it->second
- : std::shared_ptr<const PropValue>{};
- }
-
// Returns the name of the this package.
const std::string& GetName() const { return name_; }
private:
std::string name_;
- ObjectSchema types_;
- ValueMap values_;
+ base::DictionaryValue types_;
+ base::DictionaryValue values_;
friend class StatePackageTestHelper;
DISALLOW_COPY_AND_ASSIGN(StatePackage);
diff --git a/src/states/state_package_unittest.cc b/src/states/state_package_unittest.cc
index f958bf5..5e1b933 100644
--- a/src/states/state_package_unittest.cc
+++ b/src/states/state_package_unittest.cc
@@ -21,11 +21,11 @@
class StatePackageTestHelper {
public:
// Returns the state property definitions (types/constraints/etc).
- static const ObjectSchema& GetTypes(const StatePackage& package) {
+ static const base::DictionaryValue& GetTypes(const StatePackage& package) {
return package.types_;
}
// Returns the all state property values in this package.
- static const ValueMap& GetValues(const StatePackage& package) {
+ static const base::DictionaryValue& GetValues(const StatePackage& package) {
return package.values_;
}
};
@@ -33,15 +33,30 @@
namespace {
std::unique_ptr<base::DictionaryValue> GetTestSchema() {
return CreateDictionaryValue(R"({
- 'light': 'boolean',
- 'color': 'string',
- 'direction':{
- 'properties':{
- 'azimuth': {'type': 'number', 'isRequired': true},
- 'altitude': {'maximum': 90.0}
- }
+ 'color': {
+ 'type': 'string'
},
- 'iso': [50, 100, 200, 400]
+ 'direction': {
+ 'additionalProperties': false,
+ 'properties': {
+ 'altitude': {
+ 'maximum': 90.0,
+ 'type': 'number'
+ },
+ 'azimuth': {
+ 'type': 'number'
+ }
+ },
+ 'type': 'object',
+ 'required': [ 'azimuth' ]
+ },
+ 'iso': {
+ 'enum': [50, 100, 200, 400],
+ 'type': 'integer'
+ },
+ 'light': {
+ 'type': 'boolean'
+ }
})");
}
@@ -54,11 +69,11 @@
})");
}
-inline const ObjectSchema& GetTypes(const StatePackage& package) {
+inline const base::DictionaryValue& GetTypes(const StatePackage& package) {
return StatePackageTestHelper::GetTypes(package);
}
// Returns the all state property values in this package.
-inline const ValueMap& GetValues(const StatePackage& package) {
+inline const base::DictionaryValue& GetValues(const StatePackage& package) {
return StatePackageTestHelper::GetValues(package);
}
@@ -78,15 +93,15 @@
TEST(StatePackage, Empty) {
StatePackage package("test");
EXPECT_EQ("test", package.GetName());
- EXPECT_TRUE(GetTypes(package).GetProps().empty());
+ EXPECT_TRUE(GetTypes(package).empty());
EXPECT_TRUE(GetValues(package).empty());
}
TEST(StatePackage, AddSchemaFromJson_OnEmpty) {
StatePackage package("test");
ASSERT_TRUE(package.AddSchemaFromJson(GetTestSchema().get(), nullptr));
- EXPECT_EQ(4, GetTypes(package).GetProps().size());
- EXPECT_EQ(4, GetValues(package).size());
+ EXPECT_EQ(4, GetTypes(package).size());
+ EXPECT_EQ(0, GetValues(package).size());
auto expected = R"({
'color': {
@@ -114,15 +129,9 @@
'type': 'boolean'
}
})";
- EXPECT_JSON_EQ(expected, *GetTypes(package).ToJson(true, false));
+ EXPECT_JSON_EQ(expected, GetTypes(package));
- expected = R"({
- 'color': '',
- 'direction': {},
- 'iso': 0,
- 'light': false
- })";
- EXPECT_JSON_EQ(expected, *package.GetValuesAsJson());
+ EXPECT_JSON_EQ("{}", *package.GetValuesAsJson());
}
TEST(StatePackage, AddValuesFromJson_OnEmpty) {
@@ -143,10 +152,13 @@
}
TEST_F(StatePackageTest, AddSchemaFromJson_AddMore) {
- auto dict = CreateDictionaryValue("{'brightness':['low', 'medium', 'high']}");
+ auto dict = CreateDictionaryValue(R"({'brightness':{
+ 'enum': ['low', 'medium', 'high'],
+ 'type': 'string'
+ }})");
ASSERT_TRUE(package_->AddSchemaFromJson(dict.get(), nullptr));
- EXPECT_EQ(5, GetTypes(*package_).GetProps().size());
- EXPECT_EQ(5, GetValues(*package_).size());
+ EXPECT_EQ(5, GetTypes(*package_).size());
+ EXPECT_EQ(4, GetValues(*package_).size());
auto expected = R"({
'brightness': {
'enum': ['low', 'medium', 'high'],
@@ -177,10 +189,9 @@
'type': 'boolean'
}
})";
- EXPECT_JSON_EQ(expected, *GetTypes(*package_).ToJson(true, false));
+ EXPECT_JSON_EQ(expected, GetTypes(*package_));
expected = R"({
- 'brightness': '',
'color': 'white',
'direction': {
'altitude': 89.9,
@@ -193,7 +204,10 @@
}
TEST_F(StatePackageTest, AddValuesFromJson_AddMore) {
- auto dict = CreateDictionaryValue("{'brightness':['low', 'medium', 'high']}");
+ auto dict = CreateDictionaryValue(R"({'brightness':{
+ 'enum': ['low', 'medium', 'high'],
+ 'type': 'string'
+ }})");
ASSERT_TRUE(package_->AddSchemaFromJson(dict.get(), nullptr));
dict = CreateDictionaryValue("{'brightness':'medium'}");
ASSERT_TRUE(package_->AddValuesFromJson(dict.get(), nullptr));
@@ -212,7 +226,8 @@
}
TEST_F(StatePackageTest, AddSchemaFromJson_Error_Redefined) {
- auto dict = CreateDictionaryValue("{'color':['white', 'blue', 'red']}");
+ auto dict = CreateDictionaryValue(R"({'color':
+ {'type':'string', 'enum':['white', 'blue', 'red']}})");
ErrorPtr error;
EXPECT_FALSE(package_->AddSchemaFromJson(dict.get(), &error));
EXPECT_EQ(errors::state::kDomain, error->GetDomain());
@@ -221,10 +236,7 @@
TEST_F(StatePackageTest, AddValuesFromJson_Error_Undefined) {
auto dict = CreateDictionaryValue("{'brightness':'medium'}");
- ErrorPtr error;
- EXPECT_FALSE(package_->AddValuesFromJson(dict.get(), &error));
- EXPECT_EQ(errors::state::kDomain, error->GetDomain());
- EXPECT_EQ(errors::state::kPropertyNotDefined, error->GetCode());
+ EXPECT_TRUE(package_->AddValuesFromJson(dict.get(), nullptr));
}
TEST_F(StatePackageTest, GetPropertyValue) {
@@ -274,85 +286,4 @@
EXPECT_JSON_EQ(expected, *package_->GetValuesAsJson());
}
-TEST_F(StatePackageTest, SetPropertyValue_Error_TypeMismatch) {
- ErrorPtr error;
- ASSERT_FALSE(
- package_->SetPropertyValue("color", base::FundamentalValue{12}, &error));
- EXPECT_EQ(errors::commands::kDomain, error->GetDomain());
- EXPECT_EQ(errors::commands::kTypeMismatch, error->GetCode());
- error.reset();
-
- ASSERT_FALSE(
- package_->SetPropertyValue("iso", base::FundamentalValue{false}, &error));
- EXPECT_EQ(errors::commands::kDomain, error->GetDomain());
- EXPECT_EQ(errors::commands::kTypeMismatch, error->GetCode());
-}
-
-TEST_F(StatePackageTest, SetPropertyValue_Error_OutOfRange) {
- ErrorPtr error;
- ASSERT_FALSE(
- package_->SetPropertyValue("iso", base::FundamentalValue{150}, &error));
- EXPECT_EQ(errors::commands::kDomain, error->GetDomain());
- EXPECT_EQ(errors::commands::kOutOfRange, error->GetCode());
-}
-
-TEST_F(StatePackageTest, SetPropertyValue_Error_Object_TypeMismatch) {
- ErrorPtr error;
- ASSERT_FALSE(package_->SetPropertyValue(
- "direction",
- *CreateDictionaryValue("{'altitude': 45.0, 'azimuth': '15'}"), &error));
- EXPECT_EQ(errors::commands::kDomain, error->GetDomain());
- EXPECT_EQ(errors::commands::kInvalidPropValue, error->GetCode());
- const Error* inner = error->GetInnerError();
- EXPECT_EQ(errors::commands::kDomain, inner->GetDomain());
- EXPECT_EQ(errors::commands::kTypeMismatch, inner->GetCode());
-}
-
-TEST_F(StatePackageTest, SetPropertyValue_Error_Object_OutOfRange) {
- ErrorPtr error;
- ASSERT_FALSE(package_->SetPropertyValue(
- "direction",
- *CreateDictionaryValue("{'altitude': 100.0, 'azimuth': 290.0}"), &error));
- EXPECT_EQ(errors::commands::kDomain, error->GetDomain());
- EXPECT_EQ(errors::commands::kInvalidPropValue, error->GetCode());
- const Error* inner = error->GetInnerError();
- EXPECT_EQ(errors::commands::kDomain, inner->GetDomain());
- EXPECT_EQ(errors::commands::kOutOfRange, inner->GetCode());
-}
-
-TEST_F(StatePackageTest, SetPropertyValue_Error_Object_UnknownProperty) {
- ErrorPtr error;
- ASSERT_FALSE(package_->SetPropertyValue(
- "direction", *CreateDictionaryValue(
- "{'altitude': 10.0, 'azimuth': 20.0, 'spin': 30.0}"),
- &error));
- EXPECT_EQ(errors::commands::kDomain, error->GetDomain());
- 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) {
- ErrorPtr error;
- ASSERT_FALSE(package_->SetPropertyValue(
- "direction", *CreateDictionaryValue("{'altitude': 10.0}"), &error));
- EXPECT_EQ(errors::commands::kDomain, error->GetDomain());
- EXPECT_EQ(errors::commands::kPropertyMissing, error->GetCode());
-}
-
-TEST_F(StatePackageTest, SetPropertyValue_Error_Unknown) {
- ErrorPtr error;
- ASSERT_FALSE(package_->SetPropertyValue("volume", base::FundamentalValue{100},
- &error));
- EXPECT_EQ(errors::state::kDomain, error->GetDomain());
- EXPECT_EQ(errors::state::kPropertyNotDefined, error->GetCode());
-}
-
} // namespace weave
