libweave: Switch StatePackage and StateManager from Any type
Updating properties was implemented using chromeos::Any. We are removing
this with dbus references.
Also seems 'Any' version threated all properties as required so had to
update GetTestSchema in StatePackageTest.
BUG=brillo:1246
TEST='FEATURES=test emerge-gizmo buffet'
Change-Id: Ic7a2809d6d2e3f17f6993cf0fa6938c2f94cb0ec
Reviewed-on: https://chromium-review.googlesource.com/289643
Trybot-Ready: Vitaly Buka <vitalybuka@chromium.org>
Tested-by: Vitaly Buka <vitalybuka@chromium.org>
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
diff --git a/buffet/manager.cc b/buffet/manager.cc
index 1ee1793..b1070b5 100644
--- a/buffet/manager.cc
+++ b/buffet/manager.cc
@@ -23,6 +23,8 @@
#include <dbus/object_path.h>
#include <dbus/values_util.h>
+// TODO(vitalybuka): Will be moved into buffet soon.
+#include "libweave/src/commands/dbus_conversion.h"
#include "weave/enum_to_string.h"
using chromeos::dbus_utils::AsyncEventSequencer;
@@ -137,7 +139,12 @@
void Manager::UpdateState(DBusMethodResponsePtr<> response,
const chromeos::VariantDictionary& property_set) {
chromeos::ErrorPtr error;
- if (!device_->GetState()->SetProperties(property_set, &error))
+ auto properties =
+ weave::DictionaryFromDBusVariantDictionary(property_set, &error);
+ if (!properties)
+ response->ReplyWithError(error.get());
+
+ if (!device_->GetState()->SetProperties(*properties, &error))
response->ReplyWithError(error.get());
else
response->Return();
diff --git a/libweave/include/weave/state.h b/libweave/include/weave/state.h
index d34b408..c817a75 100644
--- a/libweave/include/weave/state.h
+++ b/libweave/include/weave/state.h
@@ -7,7 +7,6 @@
#include <base/callback.h>
#include <base/values.h>
-#include <chromeos/variant_dictionary.h>
namespace weave {
@@ -17,7 +16,7 @@
virtual void AddOnChangedCallback(const base::Closure& callback) = 0;
// Updates a multiple property values.
- virtual bool SetProperties(const chromeos::VariantDictionary& property_set,
+ virtual bool SetProperties(const base::DictionaryValue& property_set,
chromeos::ErrorPtr* error) = 0;
// Returns aggregated state properties across all registered packages as
diff --git a/libweave/src/base_api_handler.cc b/libweave/src/base_api_handler.cc
index 28be501..5ecc5fc 100644
--- a/libweave/src/base_api_handler.cc
+++ b/libweave/src/base_api_handler.cc
@@ -44,11 +44,13 @@
parameters->GetBoolean("localDiscoveryEnabled", &discovery_enabled);
parameters->GetBoolean("localPairingEnabled", &pairing_enabled);
- chromeos::VariantDictionary state{
- {"base.localAnonymousAccessMaxRole", anonymous_access_role},
- {"base.localDiscoveryEnabled", discovery_enabled},
- {"base.localPairingEnabled", pairing_enabled},
- };
+ base::DictionaryValue state;
+ state.SetStringWithoutPathExpansion("base.localAnonymousAccessMaxRole",
+ anonymous_access_role);
+ state.SetBooleanWithoutPathExpansion("base.localDiscoveryEnabled",
+ discovery_enabled);
+ state.SetBooleanWithoutPathExpansion("base.localPairingEnabled",
+ pairing_enabled);
if (!state_manager_->SetProperties(state, nullptr)) {
return command->Abort();
}
diff --git a/libweave/src/commands/object_schema.cc b/libweave/src/commands/object_schema.cc
index f117b6b..1aa3bfa 100644
--- a/libweave/src/commands/object_schema.cc
+++ b/libweave/src/commands/object_schema.cc
@@ -367,7 +367,7 @@
chromeos::Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain,
errors::commands::kUnknownType,
"Unexpected JSON value type: %s", type_name);
- return {};
+ return nullptr;
}
std::unique_ptr<ObjectSchema> ObjectSchema::Create() {
diff --git a/libweave/src/states/state_manager.cc b/libweave/src/states/state_manager.cc
index 850541d..0c5bad5 100644
--- a/libweave/src/states/state_manager.cc
+++ b/libweave/src/states/state_manager.cc
@@ -90,8 +90,9 @@
} else {
LOG(ERROR) << "Failed to read file for firmwareVersion.";
}
- CHECK(SetPropertyValue(kBaseStateFirmwareVersion, firmware_version,
- base::Time::Now(), nullptr));
+ CHECK(SetPropertyValue(kBaseStateFirmwareVersion,
+ base::StringValue{firmware_version}, base::Time::Now(),
+ nullptr));
for (const auto& cb : on_changed_)
cb.Run();
@@ -108,13 +109,13 @@
return dict;
}
-bool StateManager::SetProperties(
- const chromeos::VariantDictionary& property_set,
- chromeos::ErrorPtr* error) {
+bool StateManager::SetProperties(const base::DictionaryValue& property_set,
+ chromeos::ErrorPtr* error) {
base::Time timestamp = base::Time::Now();
bool all_success = true;
- for (const auto& pair : property_set) {
- if (!SetPropertyValue(pair.first, pair.second, timestamp, error)) {
+ for (base::DictionaryValue::Iterator it(property_set); !it.IsAtEnd();
+ it.Advance()) {
+ if (!SetPropertyValue(it.key(), it.value(), timestamp, error)) {
// Remember that an error occurred but keep going and update the rest of
// the properties if possible.
all_success = false;
@@ -126,7 +127,7 @@
}
bool StateManager::SetPropertyValue(const std::string& full_property_name,
- const chromeos::Any& value,
+ const base::Value& value,
const base::Time& timestamp,
chromeos::ErrorPtr* error) {
std::string package_name;
diff --git a/libweave/src/states/state_manager.h b/libweave/src/states/state_manager.h
index 8b22c2f..50a3836 100644
--- a/libweave/src/states/state_manager.h
+++ b/libweave/src/states/state_manager.h
@@ -15,7 +15,6 @@
#include <base/callback.h>
#include <base/macros.h>
#include <chromeos/errors/error.h>
-#include <chromeos/variant_dictionary.h>
#include "libweave/src/states/state_change_queue_interface.h"
#include "libweave/src/states/state_package.h"
@@ -39,7 +38,7 @@
// State overrides.
void AddOnChangedCallback(const base::Closure& callback) override;
- bool SetProperties(const chromeos::VariantDictionary& property_set,
+ bool SetProperties(const base::DictionaryValue& property_set,
chromeos::ErrorPtr* error) override;
std::unique_ptr<base::DictionaryValue> GetStateValuesAsJson() const override;
@@ -72,7 +71,7 @@
// Updates a single property value. |full_property_name| must be the full
// name of the property to update in format "package.property".
bool SetPropertyValue(const std::string& full_property_name,
- const chromeos::Any& value,
+ const base::Value& value,
const base::Time& timestamp,
chromeos::ErrorPtr* error);
diff --git a/libweave/src/states/state_manager_unittest.cc b/libweave/src/states/state_manager_unittest.cc
index 2f63c14..8325677 100644
--- a/libweave/src/states/state_manager_unittest.cc
+++ b/libweave/src/states/state_manager_unittest.cc
@@ -74,7 +74,7 @@
}
bool SetPropertyValue(const std::string& name,
- const chromeos::Any& value,
+ const base::Value& value,
chromeos::ErrorPtr* error) {
return mgr_->SetPropertyValue(name, value, timestamp_, error);
}
@@ -140,7 +140,7 @@
NotifyPropertiesUpdated(timestamp_, expected_prop_set))
.WillOnce(Return(true));
ASSERT_TRUE(SetPropertyValue("device.state_property",
- std::string{"Test Value"}, nullptr));
+ base::StringValue{"Test Value"}, nullptr));
auto expected = R"({
'base': {
'manufacturer': 'Test Factory',
@@ -155,28 +155,31 @@
TEST_F(StateManagerTest, SetPropertyValue_Error_NoName) {
chromeos::ErrorPtr error;
- ASSERT_FALSE(SetPropertyValue("", int{0}, &error));
+ ASSERT_FALSE(SetPropertyValue("", base::FundamentalValue{0}, &error));
EXPECT_EQ(errors::state::kDomain, error->GetDomain());
EXPECT_EQ(errors::state::kPropertyNameMissing, error->GetCode());
}
TEST_F(StateManagerTest, SetPropertyValue_Error_NoPackage) {
chromeos::ErrorPtr error;
- ASSERT_FALSE(SetPropertyValue("state_property", int{0}, &error));
+ ASSERT_FALSE(
+ SetPropertyValue("state_property", base::FundamentalValue{0}, &error));
EXPECT_EQ(errors::state::kDomain, error->GetDomain());
EXPECT_EQ(errors::state::kPackageNameMissing, error->GetCode());
}
TEST_F(StateManagerTest, SetPropertyValue_Error_UnknownPackage) {
chromeos::ErrorPtr error;
- ASSERT_FALSE(SetPropertyValue("power.level", int{0}, &error));
+ ASSERT_FALSE(
+ SetPropertyValue("power.level", base::FundamentalValue{0}, &error));
EXPECT_EQ(errors::state::kDomain, error->GetDomain());
EXPECT_EQ(errors::state::kPropertyNotDefined, error->GetCode());
}
TEST_F(StateManagerTest, SetPropertyValue_Error_UnknownProperty) {
chromeos::ErrorPtr error;
- ASSERT_FALSE(SetPropertyValue("base.level", int{0}, &error));
+ ASSERT_FALSE(
+ SetPropertyValue("base.level", base::FundamentalValue{0}, &error));
EXPECT_EQ(errors::state::kDomain, error->GetDomain());
EXPECT_EQ(errors::state::kPropertyNotDefined, error->GetCode());
}
@@ -185,7 +188,7 @@
EXPECT_CALL(mock_state_change_queue_, NotifyPropertiesUpdated(timestamp_, _))
.WillOnce(Return(true));
ASSERT_TRUE(SetPropertyValue("device.state_property",
- std::string{"Test Value"}, nullptr));
+ base::StringValue{"Test Value"}, nullptr));
std::vector<StateChange> expected_val;
expected_val.emplace_back(
timestamp_, ValueMap{{"device.state_property",
@@ -209,7 +212,7 @@
EXPECT_CALL(*this, OnStateChanged()).Times(1);
ASSERT_TRUE(mgr_->SetProperties(
- {{"base.manufacturer", std::string("No Name")}}, nullptr));
+ *CreateDictionaryValue("{'base.manufacturer': 'No Name'}"), nullptr));
auto expected = R"({
'base': {
diff --git a/libweave/src/states/state_package.cc b/libweave/src/states/state_package.cc
index b34fc39..170b03c 100644
--- a/libweave/src/states/state_package.cc
+++ b/libweave/src/states/state_package.cc
@@ -46,22 +46,9 @@
bool StatePackage::AddValuesFromJson(const base::DictionaryValue* json,
chromeos::ErrorPtr* error) {
- base::DictionaryValue::Iterator iter(*json);
- while (!iter.IsAtEnd()) {
- std::string property_name = iter.key();
- auto it = values_.find(property_name);
- if (it == values_.end()) {
- chromeos::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());
+ for (base::DictionaryValue::Iterator it(*json); !it.IsAtEnd(); it.Advance()) {
+ if (!SetPropertyValue(it.key(), it.value(), error))
return false;
- }
- auto new_value = it->second->GetPropType()->CreateValue();
- if (!new_value->FromJson(&iter.value(), error))
- return false;
- it->second = std::move(new_value);
- iter.Advance();
}
return true;
}
@@ -76,21 +63,23 @@
return dict;
}
-chromeos::Any StatePackage::GetPropertyValue(const std::string& property_name,
- chromeos::ErrorPtr* error) const {
+std::unique_ptr<base::Value> StatePackage::GetPropertyValue(
+ const std::string& property_name,
+ chromeos::ErrorPtr* error) const {
auto it = values_.find(property_name);
if (it == values_.end()) {
chromeos::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 chromeos::Any();
+ return nullptr;
}
- return PropValueToDBusVariant(it->second.get());
+
+ return it->second->ToJson();
}
bool StatePackage::SetPropertyValue(const std::string& property_name,
- const chromeos::Any& value,
+ const base::Value& value,
chromeos::ErrorPtr* error) {
auto it = values_.find(property_name);
if (it == values_.end()) {
@@ -100,9 +89,8 @@
name_.c_str(), property_name.c_str());
return false;
}
- auto new_value =
- PropValueFromDBusVariant(it->second->GetPropType(), value, error);
- if (!new_value)
+ auto new_value = it->second->GetPropType()->CreateValue();
+ if (!new_value->FromJson(&value, error))
return false;
it->second = std::move(new_value);
return true;
diff --git a/libweave/src/states/state_package.h b/libweave/src/states/state_package.h
index 1fceb2e..0ee7cc9 100644
--- a/libweave/src/states/state_package.h
+++ b/libweave/src/states/state_package.h
@@ -10,7 +10,6 @@
#include <string>
#include <base/macros.h>
-#include <chromeos/any.h>
#include <chromeos/errors/error.h>
#include "libweave/src/commands/object_schema.h"
@@ -56,12 +55,13 @@
// Gets the value for a specific state property. |property_name| must not
// include the package name as part of the property name.
- chromeos::Any GetPropertyValue(const std::string& property_name,
- chromeos::ErrorPtr* error) const;
+ std::unique_ptr<base::Value> GetPropertyValue(
+ const std::string& property_name,
+ chromeos::ErrorPtr* error) const;
// Sets the value for a specific state property. |property_name| must not
// include the package name as part of the property name.
bool SetPropertyValue(const std::string& property_name,
- const chromeos::Any& value,
+ const base::Value& value,
chromeos::ErrorPtr* error);
std::shared_ptr<const PropValue> GetProperty(
diff --git a/libweave/src/states/state_package_unittest.cc b/libweave/src/states/state_package_unittest.cc
index 6e76062..8900e7f 100644
--- a/libweave/src/states/state_package_unittest.cc
+++ b/libweave/src/states/state_package_unittest.cc
@@ -8,7 +8,6 @@
#include <string>
#include <base/values.h>
-#include <chromeos/variant_dictionary.h>
#include <gtest/gtest.h>
#include "libweave/src/commands/schema_constants.h"
@@ -36,7 +35,12 @@
return CreateDictionaryValue(R"({
'light': 'boolean',
'color': 'string',
- 'direction':{'properties':{'azimuth':'number','altitude':{'maximum':90.0}}},
+ 'direction':{
+ 'properties':{
+ 'azimuth': {'type': 'number', 'isRequired': true},
+ 'altitude': {'maximum': 90.0}
+ }
+ },
'iso': [50, 100, 200, 400]
})");
}
@@ -99,7 +103,8 @@
'type': 'number'
}
},
- 'type': 'object'
+ 'type': 'object',
+ 'required': [ 'azimuth' ]
},
'iso': {
'enum': [50, 100, 200, 400],
@@ -161,7 +166,8 @@
'type': 'number'
}
},
- 'type': 'object'
+ 'type': 'object',
+ 'required': [ 'azimuth' ]
},
'iso': {
'enum': [50, 100, 200, 400],
@@ -222,50 +228,39 @@
}
TEST_F(StatePackageTest, GetPropertyValue) {
- chromeos::Any value = package_->GetPropertyValue("color", nullptr);
- EXPECT_EQ("white", value.TryGet<std::string>());
-
- value = package_->GetPropertyValue("light", nullptr);
- EXPECT_TRUE(value.TryGet<bool>());
-
- value = package_->GetPropertyValue("iso", nullptr);
- EXPECT_EQ(200, value.TryGet<int>());
-
- value = package_->GetPropertyValue("direction", nullptr);
- auto direction = value.TryGet<chromeos::VariantDictionary>();
- ASSERT_FALSE(direction.empty());
- EXPECT_DOUBLE_EQ(89.9, direction["altitude"].TryGet<double>());
- EXPECT_DOUBLE_EQ(57.2957795, direction["azimuth"].TryGet<double>());
+ EXPECT_JSON_EQ("'white'", *package_->GetPropertyValue("color", nullptr));
+ EXPECT_JSON_EQ("true", *package_->GetPropertyValue("light", nullptr));
+ EXPECT_JSON_EQ("200", *package_->GetPropertyValue("iso", nullptr));
+ EXPECT_JSON_EQ("{'altitude': 89.9, 'azimuth': 57.2957795}",
+ *package_->GetPropertyValue("direction", nullptr));
}
TEST_F(StatePackageTest, GetPropertyValue_Unknown) {
chromeos::ErrorPtr error;
- chromeos::Any value = package_->GetPropertyValue("volume", &error);
- EXPECT_TRUE(value.IsEmpty());
+ EXPECT_EQ(nullptr, package_->GetPropertyValue("volume", &error));
EXPECT_EQ(errors::state::kDomain, error->GetDomain());
EXPECT_EQ(errors::state::kPropertyNotDefined, error->GetCode());
}
TEST_F(StatePackageTest, SetPropertyValue_Simple) {
EXPECT_TRUE(
- package_->SetPropertyValue("color", std::string{"blue"}, nullptr));
- chromeos::Any value = package_->GetPropertyValue("color", nullptr);
- EXPECT_EQ("blue", value.TryGet<std::string>());
-
- EXPECT_TRUE(package_->SetPropertyValue("light", bool{false}, nullptr));
- value = package_->GetPropertyValue("light", nullptr);
- EXPECT_FALSE(value.TryGet<bool>());
-
- EXPECT_TRUE(package_->SetPropertyValue("iso", int{400}, nullptr));
- value = package_->GetPropertyValue("iso", nullptr);
- EXPECT_EQ(400, value.TryGet<int>());
+ package_->SetPropertyValue("color", base::StringValue{"blue"}, nullptr));
+ EXPECT_JSON_EQ("'blue'", *package_->GetPropertyValue("color", nullptr));
+ EXPECT_TRUE(package_->SetPropertyValue("light", base::FundamentalValue{false},
+ nullptr));
+ bool light = false;
+ ASSERT_TRUE(
+ package_->GetPropertyValue("light", nullptr)->GetAsBoolean(&light));
+ EXPECT_FALSE(light);
+ EXPECT_TRUE(
+ package_->SetPropertyValue("iso", base::FundamentalValue{400}, nullptr));
+ EXPECT_JSON_EQ("400", *package_->GetPropertyValue("iso", nullptr));
}
TEST_F(StatePackageTest, SetPropertyValue_Object) {
- chromeos::VariantDictionary direction{
- {"altitude", double{45.0}}, {"azimuth", double{15.0}},
- };
- EXPECT_TRUE(package_->SetPropertyValue("direction", direction, nullptr));
+ EXPECT_TRUE(package_->SetPropertyValue(
+ "direction",
+ *CreateDictionaryValue("{'altitude': 45.0, 'azimuth': 15.0}"), nullptr));
auto expected = R"({
'color': 'white',
@@ -281,29 +276,31 @@
TEST_F(StatePackageTest, SetPropertyValue_Error_TypeMismatch) {
chromeos::ErrorPtr error;
- ASSERT_FALSE(package_->SetPropertyValue("color", int{12}, &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", bool{false}, &error));
+ 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) {
chromeos::ErrorPtr error;
- ASSERT_FALSE(package_->SetPropertyValue("iso", int{150}, &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) {
chromeos::ErrorPtr error;
- chromeos::VariantDictionary direction{
- {"altitude", double{45.0}}, {"azimuth", int{15}},
- };
- ASSERT_FALSE(package_->SetPropertyValue("direction", direction, &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 chromeos::Error* inner = error->GetInnerError();
@@ -313,10 +310,9 @@
TEST_F(StatePackageTest, SetPropertyValue_Error_Object_OutOfRange) {
chromeos::ErrorPtr error;
- chromeos::VariantDictionary direction{
- {"altitude", double{100.0}}, {"azimuth", double{290.0}},
- };
- ASSERT_FALSE(package_->SetPropertyValue("direction", direction, &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 chromeos::Error* inner = error->GetInnerError();
@@ -326,29 +322,28 @@
TEST_F(StatePackageTest, SetPropertyValue_Error_Object_UnknownProperty) {
chromeos::ErrorPtr error;
- chromeos::VariantDictionary direction{
- {"altitude", double{10.0}},
- {"azimuth", double{20.0}},
- {"spin", double{30.0}},
- };
- ASSERT_FALSE(package_->SetPropertyValue("direction", direction, &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_Error_Object_MissingProperty) {
chromeos::ErrorPtr error;
- chromeos::VariantDictionary direction{
- {"altitude", double{10.0}},
- };
- ASSERT_FALSE(package_->SetPropertyValue("direction", direction, &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());
EXPECT_EQ(errors::commands::kPropertyMissing, error->GetCode());
}
TEST_F(StatePackageTest, SetPropertyValue_Error_Unknown) {
chromeos::ErrorPtr error;
- ASSERT_FALSE(package_->SetPropertyValue("volume", int{100}, &error));
+ ASSERT_FALSE(package_->SetPropertyValue("volume", base::FundamentalValue{100},
+ &error));
EXPECT_EQ(errors::state::kDomain, error->GetDomain());
EXPECT_EQ(errors::state::kPropertyNotDefined, error->GetCode());
}
