Remove standard state definitions
They are not being validated and can be replaced by user.
BUG:25328223
Change-Id: If3afc7dac8d40a91ef98f40b89c481f59c2e5ddd
Reviewed-on: https://weave-review.googlesource.com/1423
Reviewed-by: Alex Vakulenko <avakulenko@google.com>
diff --git a/libweave/src/base_api_handler.cc b/libweave/src/base_api_handler.cc
index 52b5399..5cebb87 100644
--- a/libweave/src/base_api_handler.cc
+++ b/libweave/src/base_api_handler.cc
@@ -22,8 +22,15 @@
BaseApiHandler::BaseApiHandler(DeviceRegistrationInfo* device_info,
Device* device)
: device_info_{device_info}, device_{device} {
- device_info_->GetMutableConfig()->AddOnChangedCallback(base::Bind(
- &BaseApiHandler::OnConfigChanged, weak_ptr_factory_.GetWeakPtr()));
+ device_->AddStateDefinitionsFromJson(R"({
+ "base": {
+ "firmwareVersion": "string",
+ "localDiscoveryEnabled": "boolean",
+ "localAnonymousAccessMaxRole": [ "none", "viewer", "user" ],
+ "localPairingEnabled": "boolean"
+ }
+ })");
+ OnConfigChanged(device_->GetSettings());
const auto& settings = device_info_->GetSettings();
base::DictionaryValue state;
@@ -45,6 +52,9 @@
device_->AddCommandHandler("base.updateDeviceInfo",
base::Bind(&BaseApiHandler::UpdateDeviceInfo,
weak_ptr_factory_.GetWeakPtr()));
+
+ device_info_->GetMutableConfig()->AddOnChangedCallback(base::Bind(
+ &BaseApiHandler::OnConfigChanged, weak_ptr_factory_.GetWeakPtr()));
}
void BaseApiHandler::UpdateBaseConfiguration(
diff --git a/libweave/src/base_api_handler_unittest.cc b/libweave/src/base_api_handler_unittest.cc
index 172c8f1..565c6aa 100644
--- a/libweave/src/base_api_handler_unittest.cc
+++ b/libweave/src/base_api_handler_unittest.cc
@@ -23,6 +23,7 @@
using testing::Eq;
using testing::Invoke;
using testing::Return;
+using testing::ReturnRef;
using testing::StrictMock;
namespace weave {
@@ -37,8 +38,12 @@
command_manager_->Startup();
state_manager_ = std::make_shared<StateManager>(&mock_state_change_queue_);
- state_manager_->Startup();
+ EXPECT_CALL(device_, AddStateDefinitionsFromJson(_))
+ .WillRepeatedly(Invoke([this](const std::string& json) {
+ EXPECT_TRUE(
+ state_manager_->LoadStateDefinitionFromJson(json, nullptr));
+ }));
EXPECT_CALL(device_, SetStateProperties(_, _))
.WillRepeatedly(
Invoke(state_manager_.get(), &StateManager::SetProperties));
@@ -58,6 +63,10 @@
dev_reg_.reset(new DeviceRegistrationInfo(command_manager_, state_manager_,
std::move(config), nullptr,
&http_client_, nullptr));
+
+ EXPECT_CALL(device_, GetSettings())
+ .WillRepeatedly(ReturnRef(dev_reg_->GetSettings()));
+
handler_.reset(new BaseApiHandler{dev_reg_.get(), &device_});
}
@@ -75,6 +84,17 @@
command_manager_->FindCommand(id)->GetState());
}
+ std::unique_ptr<base::DictionaryValue> GetBaseState() {
+ auto state = state_manager_->GetState();
+ std::set<std::string> result;
+ for (base::DictionaryValue::Iterator it{*state}; !it.IsAtEnd();
+ it.Advance()) {
+ if (it.key() != "base")
+ state->Remove(it.key(), nullptr);
+ }
+ return state;
+ }
+
provider::test::MockConfigStore config_store_;
StrictMock<provider::test::MockHttpClient> http_client_;
std::unique_ptr<DeviceRegistrationInfo> dev_reg_;
@@ -106,11 +126,10 @@
'firmwareVersion': 'TEST_FIRMWARE',
'localAnonymousAccessMaxRole': 'none',
'localDiscoveryEnabled': false,
- 'localPairingEnabled': false,
- 'network': {}
+ 'localPairingEnabled': false
}
})";
- EXPECT_JSON_EQ(expected, *state_manager_->GetState());
+ EXPECT_JSON_EQ(expected, *GetBaseState());
AddCommand(R"({
'name' : 'base.updateBaseConfiguration',
@@ -128,11 +147,10 @@
'firmwareVersion': 'TEST_FIRMWARE',
'localAnonymousAccessMaxRole': 'user',
'localDiscoveryEnabled': true,
- 'localPairingEnabled': true,
- 'network': {}
+ 'localPairingEnabled': true
}
})";
- EXPECT_JSON_EQ(expected, *state_manager_->GetState());
+ EXPECT_JSON_EQ(expected, *GetBaseState());
{
Config::Transaction change{dev_reg_->GetMutableConfig()};
@@ -143,11 +161,10 @@
'firmwareVersion': 'TEST_FIRMWARE',
'localAnonymousAccessMaxRole': 'viewer',
'localDiscoveryEnabled': true,
- 'localPairingEnabled': true,
- 'network': {}
+ 'localPairingEnabled': true
}
})";
- EXPECT_JSON_EQ(expected, *state_manager_->GetState());
+ EXPECT_JSON_EQ(expected, *GetBaseState());
}
TEST_F(BaseApiHandlerTest, UpdateDeviceInfo) {
diff --git a/libweave/src/commands/standard_definitions.cc b/libweave/src/commands/standard_definitions.cc
index b6f839a..4c20984 100644
--- a/libweave/src/commands/standard_definitions.cc
+++ b/libweave/src/commands/standard_definitions.cc
@@ -39,27 +39,4 @@
}
})";
-const char kStandardStateDefs[] = R"({
- "base": {
- "firmwareVersion": "string",
- "localDiscoveryEnabled": "boolean",
- "localAnonymousAccessMaxRole": [ "none", "viewer", "user" ],
- "localPairingEnabled": "boolean",
- "network": {
- "properties": {
- "name": "string"
- }
- }
- }
-})";
-
-const char kStandardStateDefaults[] = R"({
- "base": {
- "firmwareVersion": "unknown",
- "localDiscoveryEnabled": false,
- "localAnonymousAccessMaxRole": "none",
- "localPairingEnabled": false
- }
-})";
-
} // namespace weave
diff --git a/libweave/src/commands/standard_definitions.h b/libweave/src/commands/standard_definitions.h
index eeba38a..9c9f3e1 100644
--- a/libweave/src/commands/standard_definitions.h
+++ b/libweave/src/commands/standard_definitions.h
@@ -8,8 +8,6 @@
namespace weave {
extern const char kStandardCommandDefs[];
-extern const char kStandardStateDefs[];
-extern const char kStandardStateDefaults[];
} // namespace weave
diff --git a/libweave/src/device_manager.cc b/libweave/src/device_manager.cc
index a0c6199..1b21961 100644
--- a/libweave/src/device_manager.cc
+++ b/libweave/src/device_manager.cc
@@ -37,7 +37,6 @@
command_manager_->Startup();
state_change_queue_.reset(new StateChangeQueue(kMaxStateChangeQueueSize));
state_manager_ = std::make_shared<StateManager>(state_change_queue_.get());
- state_manager_->Startup();
std::unique_ptr<Config> config{new Config{config_store}};
config->Load();
diff --git a/libweave/src/states/state_manager.cc b/libweave/src/states/state_manager.cc
index e13babc..268dd04 100644
--- a/libweave/src/states/state_manager.cc
+++ b/libweave/src/states/state_manager.cc
@@ -28,19 +28,6 @@
callback.Run(); // Force to read current state.
}
-void StateManager::Startup() {
- LOG(INFO) << "Initializing StateManager.";
-
- // Load standard device state definition.
- CHECK(LoadStandardStateDefinition(kStandardStateDefs, nullptr));
-
- // Load standard device state defaults.
- CHECK(SetPropertiesFromJson(kStandardStateDefaults, nullptr));
-
- for (const auto& cb : on_changed_)
- cb.Run();
-}
-
std::unique_ptr<base::DictionaryValue> StateManager::GetState() const {
std::unique_ptr<base::DictionaryValue> dict{new base::DictionaryValue};
for (const auto& pair : packages_) {
@@ -164,20 +151,6 @@
return true;
}
-bool StateManager::LoadStandardStateDefinition(const std::string& json,
- ErrorPtr* error) {
- std::unique_ptr<const base::DictionaryValue> dict = LoadJsonDict(json, error);
- if (!dict)
- return false;
- if (!LoadStateDefinition(*dict, error)) {
- Error::AddToPrintf(
- error, FROM_HERE, errors::kErrorDomain, errors::kSchemaError,
- "Failed to load base state definition: '%s'", json.c_str());
- return false;
- }
- return true;
-}
-
bool StateManager::SetProperties(const base::DictionaryValue& dict,
ErrorPtr* error) {
base::Time timestamp = base::Time::Now();
diff --git a/libweave/src/states/state_manager.h b/libweave/src/states/state_manager.h
index b714c51..55faf76 100644
--- a/libweave/src/states/state_manager.h
+++ b/libweave/src/states/state_manager.h
@@ -45,10 +45,6 @@
ErrorPtr* error);
std::unique_ptr<base::DictionaryValue> GetState() const;
- // Initializes the state manager and load device state fragments.
- // Called by Buffet daemon at startup.
- void Startup();
-
// Returns the recorded state changes since last time this method has been
// called.
std::pair<StateChangeQueueInterface::UpdateID, std::vector<StateChange>>
@@ -73,11 +69,6 @@
const base::Time& timestamp,
ErrorPtr* error);
- // Loads the base device state fragment JSON. This state fragment
- // defines the standard state properties from the 'base' package as defined
- // by GCD specification.
- bool LoadStandardStateDefinition(const std::string& json, ErrorPtr* error);
-
// Finds a package by its name. Returns nullptr if not found.
StatePackage* FindPackage(const std::string& package_name);
const StatePackage* FindPackage(const std::string& package_name) const;
diff --git a/libweave/src/states/state_manager_unittest.cc b/libweave/src/states/state_manager_unittest.cc
index b62dc10..3f854fe 100644
--- a/libweave/src/states/state_manager_unittest.cc
+++ b/libweave/src/states/state_manager_unittest.cc
@@ -135,19 +135,34 @@
TEST_F(StateManagerTest, Startup) {
StateManager manager(&mock_state_change_queue_);
- manager.Startup();
- ASSERT_TRUE(manager.LoadStateDefinitionFromJson(
- R"({"power": {"battery_level":"integer"}})", nullptr));
- ASSERT_TRUE(manager.SetPropertiesFromJson(
- R"({"power": {"battery_level":44}})", nullptr));
+ auto state_definition = R"({
+ "base": {
+ "firmwareVersion": "string",
+ "localDiscoveryEnabled": "boolean",
+ "localAnonymousAccessMaxRole": [ "none", "viewer", "user" ],
+ "localPairingEnabled": "boolean"
+ },
+ "power": {"battery_level":"integer"}
+ })";
+ ASSERT_TRUE(manager.LoadStateDefinitionFromJson(state_definition, nullptr));
+
+ auto state_values = R"({
+ "base": {
+ "firmwareVersion": "unknown",
+ "localDiscoveryEnabled": false,
+ "localAnonymousAccessMaxRole": "none",
+ "localPairingEnabled": false
+ },
+ "power": {"battery_level":44}
+ })";
+ ASSERT_TRUE(manager.SetPropertiesFromJson(state_values, nullptr));
auto expected = R"({
'base': {
'firmwareVersion': 'unknown',
'localAnonymousAccessMaxRole': 'none',
'localDiscoveryEnabled': false,
- 'localPairingEnabled': false,
- 'network': {}
+ 'localPairingEnabled': false
},
'power': {
'battery_level': 44
