libweave: Update "base" state on config change
This fixes initial config load when state defaults does not match
actual device config.
Also allow to have consistent state if related option was change by
buy something else that base.updateBaseConfiguration.
Moved firmwareVersion into BaseApiHandler for consistency.
BaseApiHandler store DeviceRegistrationInfo as plain pointer as former
should not out-live latter.
BUG=brillo:810
TEST='FEATURES=test emerge-gizmo buffet'
Change-Id: Idf43c90116cc5500b09d2d1295a5d082f343db8c
Reviewed-on: https://chromium-review.googlesource.com/290201
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Trybot-Ready: Vitaly Buka <vitalybuka@chromium.org>
Tested-by: Vitaly Buka <vitalybuka@chromium.org>
diff --git a/libweave/include/weave/config.h b/libweave/include/weave/config.h
index a13eee5..74aabe8 100644
--- a/libweave/include/weave/config.h
+++ b/libweave/include/weave/config.h
@@ -47,6 +47,7 @@
class Config {
public:
+ // TODO(vitalybuka): Add information to identify changed values.
using OnChangedCallback = base::Callback<void(const Settings& settings)>;
// Sets callback which is called when config is changed.
diff --git a/libweave/src/base_api_handler.cc b/libweave/src/base_api_handler.cc
index 5ecc5fc..ed85d39 100644
--- a/libweave/src/base_api_handler.cc
+++ b/libweave/src/base_api_handler.cc
@@ -11,11 +11,37 @@
namespace weave {
+namespace {
+const char kBaseStateFirmwareVersion[] = "base.firmwareVersion";
+const char kBaseStateAnonymousAccessRole[] = "base.localAnonymousAccessMaxRole";
+const char kBaseStateDiscoveryEnabled[] = "base.localDiscoveryEnabled";
+const char kBaseStatePairingEnabled[] = "base.localPairingEnabled";
+} // namespace
+
BaseApiHandler::BaseApiHandler(
- const base::WeakPtr<DeviceRegistrationInfo>& device_info,
+ DeviceRegistrationInfo* device_info,
const std::shared_ptr<StateManager>& state_manager,
const std::shared_ptr<CommandManager>& command_manager)
: device_info_{device_info}, state_manager_{state_manager} {
+ device_info_->AddOnConfigChangedCallback(base::Bind(
+ &BaseApiHandler::OnConfigChanged, weak_ptr_factory_.GetWeakPtr()));
+
+ // Populate state fields that belong to the system.
+ base::FilePath lsb_release_path("/etc/lsb-release");
+ chromeos::KeyValueStore lsb_release_store;
+ std::string firmware_version;
+ if (lsb_release_store.Load(lsb_release_path) &&
+ lsb_release_store.GetString("CHROMEOS_RELEASE_VERSION",
+ &firmware_version)) {
+ base::DictionaryValue state;
+ state.SetStringWithoutPathExpansion(kBaseStateFirmwareVersion,
+ firmware_version);
+ CHECK(state_manager_->SetProperties(state, nullptr));
+ } else {
+ LOG(ERROR) << "Failed to get CHROMEOS_RELEASE_VERSION from "
+ << lsb_release_path.value();
+ }
+
command_manager->AddOnCommandAddedCallback(base::Bind(
&BaseApiHandler::OnCommandAdded, weak_ptr_factory_.GetWeakPtr()));
}
@@ -44,17 +70,6 @@
parameters->GetBoolean("localDiscoveryEnabled", &discovery_enabled);
parameters->GetBoolean("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();
- }
-
if (!device_info_->UpdateBaseConfig(anonymous_access_role, discovery_enabled,
pairing_enabled, nullptr)) {
return command->Abort();
@@ -63,6 +78,17 @@
command->Done();
}
+void BaseApiHandler::OnConfigChanged(const Settings& settings) {
+ base::DictionaryValue state;
+ state.SetStringWithoutPathExpansion(kBaseStateAnonymousAccessRole,
+ settings.local_anonymous_access_role);
+ state.SetBooleanWithoutPathExpansion(kBaseStateDiscoveryEnabled,
+ settings.local_discovery_enabled);
+ state.SetBooleanWithoutPathExpansion(kBaseStatePairingEnabled,
+ settings.local_pairing_enabled);
+ state_manager_->SetProperties(state, nullptr);
+}
+
void BaseApiHandler::UpdateDeviceInfo(Command* command) {
command->SetProgress(base::DictionaryValue{}, nullptr);
diff --git a/libweave/src/base_api_handler.h b/libweave/src/base_api_handler.h
index 7aad3c4..c1a5f2e 100644
--- a/libweave/src/base_api_handler.h
+++ b/libweave/src/base_api_handler.h
@@ -16,6 +16,7 @@
class CommandManager;
class DeviceRegistrationInfo;
class StateManager;
+struct Settings;
// Handles commands from 'base' package.
// Objects of the class subscribe for notification from CommandManager and
@@ -25,7 +26,7 @@
// base.updateBaseConfiguration
class BaseApiHandler final {
public:
- BaseApiHandler(const base::WeakPtr<DeviceRegistrationInfo>& device_info,
+ BaseApiHandler(DeviceRegistrationInfo* device_info,
const std::shared_ptr<StateManager>& state_manager,
const std::shared_ptr<CommandManager>& command_manager);
@@ -33,8 +34,12 @@
void OnCommandAdded(Command* command);
void UpdateBaseConfiguration(Command* command);
void UpdateDeviceInfo(Command* command);
+ bool UpdateState(const std::string& anonymous_access_role,
+ bool discovery_enabled,
+ bool pairing_enabled);
+ void OnConfigChanged(const Settings& settings);
- base::WeakPtr<DeviceRegistrationInfo> device_info_;
+ DeviceRegistrationInfo* device_info_;
std::shared_ptr<StateManager> state_manager_;
base::WeakPtrFactory<BaseApiHandler> weak_ptr_factory_{this};
diff --git a/libweave/src/base_api_handler_unittest.cc b/libweave/src/base_api_handler_unittest.cc
index 46b80f7..2ecf6a1 100644
--- a/libweave/src/base_api_handler_unittest.cc
+++ b/libweave/src/base_api_handler_unittest.cc
@@ -54,8 +54,8 @@
std::unique_ptr<BuffetConfig>{new BuffetConfig{
std::unique_ptr<StorageInterface>{new MemStorage}}},
transport_, nullptr, true, nullptr));
- handler_.reset(new BaseApiHandler{
- dev_reg_->AsWeakPtr(), state_manager_, command_manager_});
+ handler_.reset(
+ new BaseApiHandler{dev_reg_.get(), state_manager_, command_manager_});
}
void LoadCommands(const std::string& command_definitions) {
@@ -101,7 +101,7 @@
}
})");
- const BuffetConfig& config{dev_reg_->GetConfig()};
+ BuffetConfig& config{*dev_reg_->GetMutableConfig()};
AddCommand(R"({
'name' : 'base.updateBaseConfiguration',
@@ -147,6 +147,21 @@
}
})";
EXPECT_JSON_EQ(expected, *state_manager_->GetStateValuesAsJson());
+
+ {
+ BuffetConfig::Transaction change{&config};
+ change.set_local_anonymous_access_role("viewer");
+ }
+ expected = R"({
+ 'base': {
+ 'firmwareVersion': '123123',
+ 'localAnonymousAccessMaxRole': 'viewer',
+ 'localDiscoveryEnabled': true,
+ 'localPairingEnabled': true,
+ 'network': {}
+ }
+ })";
+ EXPECT_JSON_EQ(expected, *state_manager_->GetStateValuesAsJson());
}
TEST_F(BaseApiHandlerTest, UpdateDeviceInfo) {
diff --git a/libweave/src/device_manager.cc b/libweave/src/device_manager.cc
index bc83a04..cd79a02 100644
--- a/libweave/src/device_manager.cc
+++ b/libweave/src/device_manager.cc
@@ -62,8 +62,8 @@
command_manager_, state_manager_, std::move(config), transport,
base::MessageLoop::current()->task_runner(), options.xmpp_enabled,
network));
- base_api_handler_.reset(new BaseApiHandler{device_info_->AsWeakPtr(),
- state_manager_, command_manager_});
+ base_api_handler_.reset(
+ new BaseApiHandler{device_info_.get(), state_manager_, command_manager_});
device_info_->Start();
diff --git a/libweave/src/device_registration_info.h b/libweave/src/device_registration_info.h
index 9ea8e9d..82fe399 100644
--- a/libweave/src/device_registration_info.h
+++ b/libweave/src/device_registration_info.h
@@ -149,13 +149,13 @@
const BuffetConfig& GetConfig() const { return *config_; }
BuffetConfig* GetMutableConfig() { return config_.get(); }
+ private:
+ friend class DeviceRegistrationInfoTest;
+
base::WeakPtr<DeviceRegistrationInfo> AsWeakPtr() {
return weak_factory_.GetWeakPtr();
}
- private:
- friend class DeviceRegistrationInfoTest;
-
// Cause DeviceRegistrationInfo to attempt to connect to cloud server on
// its own later.
void ScheduleCloudConnection(const base::TimeDelta& delay);
diff --git a/libweave/src/states/state_manager.cc b/libweave/src/states/state_manager.cc
index 0c5bad5..522a2ec 100644
--- a/libweave/src/states/state_manager.cc
+++ b/libweave/src/states/state_manager.cc
@@ -18,12 +18,6 @@
namespace weave {
-namespace {
-
-const char kBaseStateFirmwareVersion[] = "base.firmwareVersion";
-
-} // namespace
-
StateManager::StateManager(StateChangeQueueInterface* state_change_queue)
: state_change_queue_(state_change_queue) {
CHECK(state_change_queue_) << "State change queue not specified";
@@ -78,22 +72,6 @@
json_file_path = enumerator2.Next();
}
- // Populate state fields that belong to the system.
- base::FilePath lsb_release_path("/etc/lsb-release");
- chromeos::KeyValueStore lsb_release_store;
- std::string firmware_version;
- if (lsb_release_store.Load(lsb_release_path)) {
- if (!lsb_release_store.GetString("CHROMEOS_RELEASE_VERSION",
- &firmware_version))
- LOG(ERROR) << "Missing key for firmware version in version file.";
-
- } else {
- LOG(ERROR) << "Failed to read file for firmwareVersion.";
- }
- CHECK(SetPropertyValue(kBaseStateFirmwareVersion,
- base::StringValue{firmware_version}, base::Time::Now(),
- nullptr));
-
for (const auto& cb : on_changed_)
cb.Run();
}
