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