buffet: Move config and buffet state logic into BuffetConfig Load/Save logic isolated in buffet/buffet_config.* files. Added BuffetConfig::Change helper to make sure callbacks were called and changes were saved. BUG=brillo:1058 TEST='FEATURES=test emerge-gizmo buffet' Change-Id: Id8f171c2109fe834daef43658abf6881b50b5c7d Reviewed-on: https://chromium-review.googlesource.com/271343 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/base_api_handler_unittest.cc b/buffet/base_api_handler_unittest.cc index 011f639..d1de49f 100644 --- a/buffet/base_api_handler_unittest.cc +++ b/buffet/base_api_handler_unittest.cc
@@ -22,15 +22,15 @@ class BaseApiHandlerTest : public ::testing::Test { protected: void SetUp() override { - storage_ = std::make_shared<MemStorage>(); transport_ = std::make_shared<chromeos::http::fake::Transport>(); command_manager_ = std::make_shared<CommandManager>(); state_manager_ = std::make_shared<StateManager>(&mock_state_change_queue_); state_manager_->Startup(); dev_reg_.reset(new DeviceRegistrationInfo( command_manager_, state_manager_, - std::unique_ptr<BuffetConfig>{new BuffetConfig}, transport_, storage_, - true, nullptr)); + std::unique_ptr<BuffetConfig>{new BuffetConfig{ + std::unique_ptr<StorageInterface>{new MemStorage}}}, + transport_, true)); handler_.reset(new BaseApiHandler{dev_reg_->AsWeakPtr(), command_manager_}); } @@ -54,7 +54,6 @@ command_manager_->FindCommand(id)->GetStatus()); } - std::shared_ptr<MemStorage> storage_; std::shared_ptr<chromeos::http::fake::Transport> transport_; std::unique_ptr<DeviceRegistrationInfo> dev_reg_; std::shared_ptr<CommandManager> command_manager_;
diff --git a/buffet/buffet.gyp b/buffet/buffet.gyp index 7a466d7..a68d772 100644 --- a/buffet/buffet.gyp +++ b/buffet/buffet.gyp
@@ -108,6 +108,7 @@ 'sources': [ 'base_api_handler_unittest.cc', 'buffet_testrunner.cc', + 'buffet_config_unittest.cc', 'commands/command_definition_unittest.cc', 'commands/command_dictionary_unittest.cc', 'commands/command_instance_unittest.cc',
diff --git a/buffet/buffet_config.cc b/buffet/buffet_config.cc index a97b654..4d97098 100644 --- a/buffet/buffet_config.cc +++ b/buffet/buffet_config.cc
@@ -7,6 +7,9 @@ #include <base/logging.h> #include <base/strings/string_number_conversions.h> +#include "buffet/storage_impls.h" +#include "buffet/storage_interface.h" + namespace { // TODO(vitalybuka): Remove this when deviceKind is gone from server. @@ -64,9 +67,21 @@ const char kModelName[] = "model_name"; const char kModelId[] = "model_id"; const char kPollingPeriodMs[] = "polling_period_ms"; +const char kRefreshToken[] = "refresh_token"; +const char kDeviceId[] = "device_id"; +const char kRobotAccount[] = "robot_account"; } // namespace config_keys +BuffetConfig::BuffetConfig(std::unique_ptr<StorageInterface> storage) + : storage_{std::move(storage)} { +} + +BuffetConfig::BuffetConfig(const base::FilePath& state_path) + : BuffetConfig{ + std::unique_ptr<StorageInterface>{new FileStorage{state_path}}} { +} + void BuffetConfig::Load(const base::FilePath& config_path) { chromeos::KeyValueStore store; if (store.Load(config_path)) { @@ -75,6 +90,9 @@ } void BuffetConfig::Load(const chromeos::KeyValueStore& store) { + Transaction change{this}; + change.save_ = false; + store.GetString(config_keys::kClientId, &client_id_); CHECK(!client_id_.empty()); @@ -117,24 +135,105 @@ store.GetBoolean(config_keys::kLocalDiscoveryEnabled, &local_discovery_enabled_); store.GetBoolean(config_keys::kLocalPairingEnabled, &local_pairing_enabled_); + + change.LoadState(); } -bool BuffetConfig::set_name(const std::string& name) { +void BuffetConfig::Transaction::LoadState() { + if (!config_->storage_) + return; + auto value = config_->storage_->Load(); + const base::DictionaryValue* dict = nullptr; + if (!value || !value->GetAsDictionary(&dict)) + return; + + std::string name; + if (dict->GetString(config_keys::kName, &name)) + set_name(name); + + std::string description; + if (dict->GetString(config_keys::kDescription, &description)) + set_description(description); + + std::string location; + if (dict->GetString(config_keys::kLocation, &location)) + set_location(location); + + std::string access_role; + if (dict->GetString(config_keys::kLocalAnonymousAccessRole, &access_role)) + set_local_anonymous_access_role(access_role); + + bool discovery_enabled{false}; + if (dict->GetBoolean(config_keys::kLocalDiscoveryEnabled, &discovery_enabled)) + set_local_discovery_enabled(discovery_enabled); + + bool pairing_enabled{false}; + if (dict->GetBoolean(config_keys::kLocalPairingEnabled, &pairing_enabled)) + set_local_pairing_enabled(pairing_enabled); + + std::string token; + if (dict->GetString(config_keys::kRefreshToken, &token)) + set_refresh_token(token); + + std::string account; + if (dict->GetString(config_keys::kRobotAccount, &account)) + set_robot_account(account); + + std::string device_id; + if (dict->GetString(config_keys::kDeviceId, &device_id)) + set_device_id(device_id); +} + +bool BuffetConfig::Save() { + if (!storage_) + return false; + base::DictionaryValue dict; + dict.SetString(config_keys::kRefreshToken, refresh_token_); + dict.SetString(config_keys::kDeviceId, device_id_); + dict.SetString(config_keys::kRobotAccount, robot_account_); + dict.SetString(config_keys::kName, name_); + dict.SetString(config_keys::kDescription, description_); + dict.SetString(config_keys::kLocation, location_); + dict.SetString(config_keys::kLocalAnonymousAccessRole, + local_anonymous_access_role_); + dict.SetBoolean(config_keys::kLocalDiscoveryEnabled, + local_discovery_enabled_); + dict.SetBoolean(config_keys::kLocalPairingEnabled, local_pairing_enabled_); + + return storage_->Save(dict); +} + +BuffetConfig::Transaction::~Transaction() { + Commit(); +} + +bool BuffetConfig::Transaction::set_name(const std::string& name) { if (name.empty()) { LOG(ERROR) << "Invalid name: " << name; return false; } - name_ = name; + config_->name_ = name; return true; } -bool BuffetConfig::set_local_anonymous_access_role(const std::string& role) { +bool BuffetConfig::Transaction::set_local_anonymous_access_role( + const std::string& role) { if (!IsValidAccessRole(role)) { LOG(ERROR) << "Invalid role: " << role; return false; } - local_anonymous_access_role_ = role; + config_->local_anonymous_access_role_ = role; return true; } +void BuffetConfig::Transaction::Commit() { + if (!config_) + return; + if (save_) + config_->Save(); + for (const auto& cb : config_->on_changed_) + cb.Run(*config_); + config_ = nullptr; +} + } // namespace buffet
diff --git a/buffet/buffet_config.h b/buffet/buffet_config.h index 0ebdf8e..0470755 100644 --- a/buffet/buffet_config.h +++ b/buffet/buffet_config.h
@@ -6,19 +6,78 @@ #define BUFFET_BUFFET_CONFIG_H_ #include <string> +#include <vector> +#include <base/callback.h> #include <base/files/file_path.h> +#include <chromeos/errors/error.h> #include <chromeos/key_value_store.h> namespace buffet { -class BuffetConfig { +class StorageInterface; + +// Handles reading buffet config and state files. +class BuffetConfig final { public: - BuffetConfig() = default; + using OnChangedCallback = base::Callback<void(const BuffetConfig&)>; + + explicit BuffetConfig(std::unique_ptr<StorageInterface> storage); + + explicit BuffetConfig(const base::FilePath& state_path); + + void AddOnChangedCallback(const OnChangedCallback& callback) { + on_changed_.push_back(callback); + // Force to read current state. + callback.Run(*this); + } void Load(const base::FilePath& config_path); void Load(const chromeos::KeyValueStore& store); + // Allows editing of config. Makes sure that callbacks were called and changes + // were saved. + // User can commit changes by calling Commit method or by destroying the + // object. + class Transaction final { + public: + explicit Transaction(BuffetConfig* config) : config_(config) { + CHECK(config_); + } + + ~Transaction(); + + bool set_name(const std::string& name); + void set_description(const std::string& description) { + config_->description_ = description; + } + void set_location(const std::string& location) { + config_->location_ = location; + } + bool set_local_anonymous_access_role(const std::string& role); + void set_local_discovery_enabled(bool enabled) { + config_->local_discovery_enabled_ = enabled; + } + void set_local_pairing_enabled(bool enabled) { + config_->local_pairing_enabled_ = enabled; + } + void set_device_id(const std::string& id) { config_->device_id_ = id; } + void set_refresh_token(const std::string& token) { + config_->refresh_token_ = token; + } + void set_robot_account(const std::string& account) { + config_->robot_account_ = account; + } + + void Commit(); + + private: + friend class BuffetConfig; + void LoadState(); + BuffetConfig* config_; + bool save_{true}; + }; + const std::string& client_id() const { return client_id_; } const std::string& client_secret() const { return client_secret_; } const std::string& api_key() const { return api_key_; } @@ -39,21 +98,13 @@ bool local_pairing_enabled() const { return local_pairing_enabled_; } bool local_discovery_enabled() const { return local_discovery_enabled_; } - bool set_name(const std::string& name); - void set_description(const std::string& description) { - description_ = description; - } - void set_location(const std::string& location) { location_ = location; } - - bool set_local_anonymous_access_role(const std::string& role); - void set_local_discovery_enabled(bool enabled) { - local_discovery_enabled_ = enabled; - } - void set_local_pairing_enabled(bool enabled) { - local_pairing_enabled_ = enabled; - } + std::string device_id() const { return device_id_; } + std::string refresh_token() const { return refresh_token_; } + std::string robot_account() const { return robot_account_; } private: + bool Save(); + std::string client_id_{"58855907228.apps.googleusercontent.com"}; std::string client_secret_{"eHSAREAHrIqPsHBxCE9zPPBi"}; std::string api_key_{"AIzaSyDSq46gG-AxUnC3zoqD9COIPrjolFsMfMA"}; @@ -71,6 +122,15 @@ std::string device_kind_{"vendor"}; uint64_t polling_period_ms_{7000}; + std::string device_id_; + std::string refresh_token_; + std::string robot_account_; + + // Serialization interface to save and load buffet state. + std::unique_ptr<StorageInterface> storage_; + + std::vector<OnChangedCallback> on_changed_; + DISALLOW_COPY_AND_ASSIGN(BuffetConfig); };
diff --git a/buffet/buffet_config_unittest.cc b/buffet/buffet_config_unittest.cc new file mode 100644 index 0000000..cec8bbe --- /dev/null +++ b/buffet/buffet_config_unittest.cc
@@ -0,0 +1,247 @@ +// Copyright 2014 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "buffet/buffet_config.h" + +#include <base/bind.h> +#include <gmock/gmock.h> +#include <gtest/gtest.h> + +#include "buffet/commands/unittest_utils.h" +#include "buffet/storage_impls.h" + +using testing::_; + +namespace buffet { + +class BuffetConfigTest : public ::testing::Test { + protected: + void SetUp() override { + EXPECT_CALL(*this, OnConfigChanged(_)) + .Times(1); // Called from AddOnChangedCallback + + std::unique_ptr<StorageInterface> storage{new MemStorage}; + storage_ = storage.get(); + config_.reset(new BuffetConfig{std::move(storage)}); + + config_->AddOnChangedCallback( + base::Bind(&BuffetConfigTest::OnConfigChanged, base::Unretained(this))); + } + + MOCK_METHOD1(OnConfigChanged, void(const BuffetConfig&)); + + StorageInterface* storage_{nullptr}; + std::unique_ptr<BuffetConfig> config_; + const BuffetConfig default_{nullptr}; +}; + +TEST_F(BuffetConfigTest, NoStorage) { + BuffetConfig config{nullptr}; + BuffetConfig::Transaction change{&config}; + change.Commit(); +} + +TEST_F(BuffetConfigTest, Defaults) { + EXPECT_EQ("58855907228.apps.googleusercontent.com", config_->client_id()); + EXPECT_EQ("eHSAREAHrIqPsHBxCE9zPPBi", config_->client_secret()); + EXPECT_EQ("AIzaSyDSq46gG-AxUnC3zoqD9COIPrjolFsMfMA", config_->api_key()); + EXPECT_EQ("https://accounts.google.com/o/oauth2/", config_->oauth_url()); + EXPECT_EQ("https://www.googleapis.com/clouddevices/v1/", + config_->service_url()); + EXPECT_EQ("Chromium", config_->oem_name()); + EXPECT_EQ("Brillo", config_->model_name()); + EXPECT_EQ("AAAAA", config_->model_id()); + EXPECT_EQ("vendor", config_->device_kind()); + EXPECT_EQ(7000, config_->polling_period_ms()); + EXPECT_EQ("Developer device", config_->name()); + EXPECT_EQ("", config_->description()); + EXPECT_EQ("", config_->location()); + EXPECT_EQ("viewer", config_->local_anonymous_access_role()); + EXPECT_TRUE(config_->local_pairing_enabled()); + EXPECT_TRUE(config_->local_discovery_enabled()); + EXPECT_EQ("", config_->device_id()); + EXPECT_EQ("", config_->refresh_token()); + EXPECT_EQ("", config_->robot_account()); +} + +TEST_F(BuffetConfigTest, LoadConfig) { + chromeos::KeyValueStore config_store; + config_store.SetString("client_id", "conf_client_id"); + config_store.SetString("client_secret", "conf_client_secret"); + config_store.SetString("api_key", "conf_api_key"); + config_store.SetString("oauth_url", "conf_oauth_url"); + config_store.SetString("service_url", "conf_service_url"); + config_store.SetString("oem_name", "conf_oem_name"); + config_store.SetString("model_name", "conf_model_name"); + config_store.SetString("model_id", "ABCDE"); + config_store.SetString("polling_period_ms", "12345"); + config_store.SetString("name", "conf_name"); + config_store.SetString("description", "conf_description"); + config_store.SetString("location", "conf_location"); + config_store.SetString("local_anonymous_access_role", "user"); + config_store.SetBoolean("local_pairing_enabled", false); + config_store.SetBoolean("local_discovery_enabled", false); + + // Following will be ignored. + config_store.SetString("device_kind", "conf_device_kind"); + config_store.SetString("device_id", "conf_device_id"); + config_store.SetString("refresh_token", "conf_refresh_token"); + config_store.SetString("robot_account", "conf_robot_account"); + + EXPECT_CALL(*this, OnConfigChanged(_)).Times(1); + config_->Load(config_store); + + EXPECT_EQ("conf_client_id", config_->client_id()); + EXPECT_EQ("conf_client_secret", config_->client_secret()); + EXPECT_EQ("conf_api_key", config_->api_key()); + EXPECT_EQ("conf_oauth_url", config_->oauth_url()); + EXPECT_EQ("conf_service_url", config_->service_url()); + EXPECT_EQ("conf_oem_name", config_->oem_name()); + EXPECT_EQ("conf_model_name", config_->model_name()); + EXPECT_EQ("ABCDE", config_->model_id()); + EXPECT_EQ("developmentBoard", config_->device_kind()); + EXPECT_EQ(12345, config_->polling_period_ms()); + EXPECT_EQ("conf_name", config_->name()); + EXPECT_EQ("conf_description", config_->description()); + EXPECT_EQ("conf_location", config_->location()); + EXPECT_EQ("user", config_->local_anonymous_access_role()); + EXPECT_FALSE(config_->local_pairing_enabled()); + EXPECT_FALSE(config_->local_discovery_enabled()); + + // Not from config. + EXPECT_EQ(default_.device_id(), config_->device_id()); + EXPECT_EQ(default_.refresh_token(), config_->refresh_token()); + EXPECT_EQ(default_.robot_account(), config_->robot_account()); + + // Nothing should be saved yet. + EXPECT_JSON_EQ("{}", *storage_->Load()); + + BuffetConfig::Transaction change{config_.get()}; + EXPECT_CALL(*this, OnConfigChanged(_)).Times(1); + change.Commit(); + + auto expected = R"({ + 'description': 'conf_description', + 'device_id': '', + 'local_anonymous_access_role': 'user', + 'local_discovery_enabled': false, + 'local_pairing_enabled': false, + 'location': 'conf_location', + 'name': 'conf_name', + 'refresh_token': '', + 'robot_account': '' + })"; + EXPECT_JSON_EQ(expected, *storage_->Load()); +} + +TEST_F(BuffetConfigTest, LoadState) { + auto state = R"({ + 'description': 'state_description', + 'device_id': 'state_device_id', + 'local_anonymous_access_role': 'user', + 'local_discovery_enabled': false, + 'local_pairing_enabled': false, + 'location': 'state_location', + 'name': 'state_name', + 'refresh_token': 'state_refresh_token', + 'robot_account': 'state_robot_account' + })"; + storage_->Save(*buffet::unittests::CreateDictionaryValue(state)); + + chromeos::KeyValueStore config_store; + EXPECT_CALL(*this, OnConfigChanged(_)).Times(1); + config_->Load(config_store); + + // Clear storage. + storage_->Save(base::DictionaryValue()); + + EXPECT_EQ(default_.client_id(), config_->client_id()); + EXPECT_EQ(default_.client_secret(), config_->client_secret()); + EXPECT_EQ(default_.api_key(), config_->api_key()); + EXPECT_EQ(default_.oauth_url(), config_->oauth_url()); + EXPECT_EQ(default_.service_url(), config_->service_url()); + EXPECT_EQ(default_.oem_name(), config_->oem_name()); + EXPECT_EQ(default_.model_name(), config_->model_name()); + EXPECT_EQ(default_.model_id(), config_->model_id()); + EXPECT_EQ(default_.device_kind(), config_->device_kind()); + EXPECT_EQ(default_.polling_period_ms(), config_->polling_period_ms()); + EXPECT_EQ("state_name", config_->name()); + EXPECT_EQ("state_description", config_->description()); + EXPECT_EQ("state_location", config_->location()); + EXPECT_EQ("user", config_->local_anonymous_access_role()); + EXPECT_FALSE(config_->local_pairing_enabled()); + EXPECT_FALSE(config_->local_discovery_enabled()); + EXPECT_EQ("state_device_id", config_->device_id()); + EXPECT_EQ("state_refresh_token", config_->refresh_token()); + EXPECT_EQ("state_robot_account", config_->robot_account()); + + // Nothing should be saved yet. + EXPECT_JSON_EQ("{}", *storage_->Load()); + + BuffetConfig::Transaction change{config_.get()}; + EXPECT_CALL(*this, OnConfigChanged(_)).Times(1); + change.Commit(); + + EXPECT_JSON_EQ(state, *storage_->Load()); +} + +TEST_F(BuffetConfigTest, Setters) { + BuffetConfig::Transaction change{config_.get()}; + + change.set_name("set_name"); + EXPECT_EQ("set_name", config_->name()); + + change.set_description("set_description"); + EXPECT_EQ("set_description", config_->description()); + + change.set_location("set_location"); + EXPECT_EQ("set_location", config_->location()); + + change.set_local_anonymous_access_role("viewer"); + EXPECT_EQ("viewer", config_->local_anonymous_access_role()); + + change.set_local_anonymous_access_role("none"); + EXPECT_EQ("none", config_->local_anonymous_access_role()); + + change.set_local_anonymous_access_role("user"); + EXPECT_EQ("user", config_->local_anonymous_access_role()); + + change.set_local_discovery_enabled(false); + EXPECT_FALSE(config_->local_discovery_enabled()); + + change.set_local_pairing_enabled(false); + EXPECT_FALSE(config_->local_pairing_enabled()); + + change.set_local_discovery_enabled(true); + EXPECT_TRUE(config_->local_discovery_enabled()); + + change.set_local_pairing_enabled(true); + EXPECT_TRUE(config_->local_pairing_enabled()); + + change.set_device_id("set_id"); + EXPECT_EQ("set_id", config_->device_id()); + + change.set_refresh_token("set_token"); + EXPECT_EQ("set_token", config_->refresh_token()); + + change.set_robot_account("set_account"); + EXPECT_EQ("set_account", config_->robot_account()); + + EXPECT_CALL(*this, OnConfigChanged(_)).Times(1); + change.Commit(); + + auto expected = R"({ + 'description': 'set_description', + 'device_id': 'set_id', + 'local_anonymous_access_role': 'user', + 'local_discovery_enabled': true, + 'local_pairing_enabled': true, + 'location': 'set_location', + 'name': 'set_name', + 'refresh_token': 'set_token', + 'robot_account': 'set_account' + })"; + EXPECT_JSON_EQ(expected, *storage_->Load()); +} +} // namespace buffet
diff --git a/buffet/device_registration_info.cc b/buffet/device_registration_info.cc index e9f806b..3b176c3 100644 --- a/buffet/device_registration_info.cc +++ b/buffet/device_registration_info.cc
@@ -28,7 +28,6 @@ #include "buffet/commands/schema_constants.h" #include "buffet/device_registration_storage_keys.h" #include "buffet/notification/xmpp_channel.h" -#include "buffet/org.chromium.Buffet.Manager.h" #include "buffet/states/state_manager.h" #include "buffet/utils.h" @@ -133,17 +132,12 @@ const std::shared_ptr<StateManager>& state_manager, std::unique_ptr<BuffetConfig> config, const std::shared_ptr<chromeos::http::Transport>& transport, - const std::shared_ptr<StorageInterface>& state_store, - bool notifications_enabled, - org::chromium::Buffet::ManagerAdaptor* manager) + bool notifications_enabled) : transport_{transport}, - storage_{state_store}, command_manager_{command_manager}, state_manager_{state_manager}, config_{std::move(config)}, - notifications_enabled_{notifications_enabled}, - manager_{manager} { - OnConfigChanged(); + notifications_enabled_{notifications_enabled} { command_manager_->AddOnCommandDefChanged( base::Bind(&DeviceRegistrationInfo::OnCommandDefsChanged, weak_factory_.GetWeakPtr())); @@ -165,9 +159,9 @@ std::string DeviceRegistrationInfo::GetDeviceURL( const std::string& subpath, const chromeos::data_encoding::WebParamList& params) const { - CHECK(!device_id_.empty()) << "Must have a valid device ID"; + CHECK(!config_->device_id().empty()) << "Must have a valid device ID"; return BuildURL(config_->service_url(), - {"devices", device_id_, subpath}, params); + {"devices", config_->device_id(), subpath}, params); } std::string DeviceRegistrationInfo::GetOAuthURL( @@ -176,56 +170,7 @@ return BuildURL(config_->oauth_url(), {subpath}, params); } -const std::string& DeviceRegistrationInfo::GetDeviceId() const { - return device_id_; -} - -bool DeviceRegistrationInfo::Load() { - // Set kInvalidCredentials to trigger on_status_changed_ callback. - registration_status_ = RegistrationStatus::kInvalidCredentials; - SetRegistrationStatus(RegistrationStatus::kUnconfigured); - - auto value = storage_->Load(); - const base::DictionaryValue* dict = nullptr; - if (!value || !value->GetAsDictionary(&dict)) - return false; - - // Read all available data before failing. - std::string name; - if (dict->GetString(storage_keys::kName, &name)) - config_->set_name(name); - - std::string description; - if (dict->GetString(storage_keys::kDescription, &description)) - config_->set_description(description); - - std::string location; - if (dict->GetString(storage_keys::kLocation, &location)) - config_->set_location(location); - - std::string access_role; - if (dict->GetString(storage_keys::kLocalAnonymousAccessRole, &access_role)) - config_->set_local_anonymous_access_role(access_role); - - bool local_discovery_enabled{false}; - if (dict->GetBoolean(storage_keys::kLocalDiscoveryEnabled, - &local_discovery_enabled)) - config_->set_local_discovery_enabled(local_discovery_enabled); - - bool local_pairing_enabled{false}; - if (dict->GetBoolean(storage_keys::kLocalPairingEnabled, - &local_pairing_enabled)) - config_->set_local_pairing_enabled(local_pairing_enabled); - - dict->GetString(storage_keys::kRefreshToken, &refresh_token_); - dict->GetString(storage_keys::kRobotAccount, &device_robot_account_); - - std::string device_id; - if (dict->GetString(storage_keys::kDeviceId, &device_id)) - SetDeviceId(device_id); - - OnConfigChanged(); - +void DeviceRegistrationInfo::Start() { if (HaveRegistrationCredentials(nullptr)) { // Wait a significant amount of time for local daemons to publish their // state to Buffet before publishing it to the cloud. @@ -235,25 +180,6 @@ // we need not wait for them. ScheduleStartDevice(base::TimeDelta::FromSeconds(5)); } - return true; -} - -bool DeviceRegistrationInfo::Save() const { - base::DictionaryValue dict; - dict.SetString(storage_keys::kRefreshToken, refresh_token_); - dict.SetString(storage_keys::kDeviceId, device_id_); - dict.SetString(storage_keys::kRobotAccount, device_robot_account_); - dict.SetString(storage_keys::kName, config_->name()); - dict.SetString(storage_keys::kDescription, config_->description()); - dict.SetString(storage_keys::kLocation, config_->location()); - dict.SetString(storage_keys::kLocalAnonymousAccessRole, - config_->local_anonymous_access_role()); - dict.SetBoolean(storage_keys::kLocalDiscoveryEnabled, - config_->local_discovery_enabled()); - dict.SetBoolean(storage_keys::kLocalPairingEnabled, - config_->local_pairing_enabled()); - - return storage_->Save(dict); } void DeviceRegistrationInfo::ScheduleStartDevice(const base::TimeDelta& later) { @@ -283,9 +209,9 @@ bool DeviceRegistrationInfo::HaveRegistrationCredentials( chromeos::ErrorPtr* error) { - const bool have_credentials = !refresh_token_.empty() && - !device_id_.empty() && - !device_robot_account_.empty(); + const bool have_credentials = !config_->refresh_token().empty() && + !config_->device_id().empty() && + !config_->robot_account().empty(); VLOG(1) << "Device registration record " << ((have_credentials) ? "found" : "not found."); @@ -336,12 +262,15 @@ bool DeviceRegistrationInfo::RefreshAccessToken( chromeos::ErrorPtr* error) { LOG(INFO) << "Refreshing access token."; - auto response = chromeos::http::PostFormDataAndBlock(GetOAuthURL("token"), { - {"refresh_token", refresh_token_}, - {"client_id", config_->client_id()}, - {"client_secret", config_->client_secret()}, - {"grant_type", "refresh_token"}, - }, {}, transport_, error); + auto response = chromeos::http::PostFormDataAndBlock( + GetOAuthURL("token"), + { + {"refresh_token", config_->refresh_token()}, + {"client_id", config_->client_id()}, + {"client_secret", config_->client_secret()}, + {"grant_type", "refresh_token"}, + }, + {}, transport_, error); if (!response) return false; @@ -385,11 +314,18 @@ // this class completely. Also to be added the secondary (poll) notification // channel. primary_notification_channel_.reset( - new XmppChannel{device_robot_account_, access_token_, + new XmppChannel{config_->robot_account(), + access_token_, base::MessageLoop::current()->task_runner()}); primary_notification_channel_->Start(this); } +void DeviceRegistrationInfo::AddOnRegistrationChangedCallback( + const OnRegistrationChangedCallback& callback) { + on_registration_changed_.push_back(callback); + callback.Run(registration_status_); +} + std::unique_ptr<base::DictionaryValue> DeviceRegistrationInfo::BuildDeviceResource(chromeos::ErrorPtr* error) { // Limit only to commands that are visible to the cloud. @@ -405,8 +341,8 @@ return nullptr; std::unique_ptr<base::DictionaryValue> resource{new base::DictionaryValue}; - if (!device_id_.empty()) - resource->SetString("id", device_id_); + if (!config_->device_id().empty()) + resource->SetString("id", config_->device_id()); resource->SetString("name", config_->name()); if (!config_->description().empty()) resource->SetString("description", config_->description()); @@ -534,7 +470,8 @@ std::string auth_code; std::string device_id; - if (!json_resp->GetString("robotAccountEmail", &device_robot_account_) || + std::string robot_account; + if (!json_resp->GetString("robotAccountEmail", &robot_account) || !json_resp->GetString("robotAccountAuthorizationCode", &auth_code) || !json_resp->GetString("deviceDraft.id", &device_id)) { chromeos::Error::AddTo(error, FROM_HERE, kErrorDomainGCD, @@ -542,7 +479,6 @@ "Device account missing in response"); return std::string(); } - SetDeviceId(device_id); // Now get access_token and refresh_token response = chromeos::http::PostFormDataAndBlock(GetOAuthURL("token"), { @@ -558,13 +494,11 @@ json_resp = ParseOAuthResponse(response.get(), error); int expires_in = 0; - if (!json_resp || - !json_resp->GetString("access_token", &access_token_) || - !json_resp->GetString("refresh_token", &refresh_token_) || + std::string refresh_token; + if (!json_resp || !json_resp->GetString("access_token", &access_token_) || + !json_resp->GetString("refresh_token", &refresh_token) || !json_resp->GetInteger("expires_in", &expires_in) || - access_token_.empty() || - refresh_token_.empty() || - expires_in <= 0) { + access_token_.empty() || refresh_token.empty() || expires_in <= 0) { chromeos::Error::AddTo(error, FROM_HERE, kErrorDomainGCD, "unexpected_response", "Device access_token missing in response"); @@ -574,13 +508,18 @@ access_token_expiration_ = base::Time::Now() + base::TimeDelta::FromSeconds(expires_in); - Save(); + BuffetConfig::Transaction change{config_.get()}; + change.set_device_id(device_id); + change.set_robot_account(robot_account); + change.set_refresh_token(refresh_token); + change.Commit(); + StartNotificationChannel(); // We're going to respond with our success immediately and we'll StartDevice // shortly after. ScheduleStartDevice(base::TimeDelta::FromSeconds(0)); - return device_id_; + return device_id; } namespace { @@ -769,17 +708,14 @@ const std::string& description, const std::string& location, chromeos::ErrorPtr* error) { - if (!config_->set_name(name)) { - chromeos::Error::AddToPrintf(error, FROM_HERE, kErrorDomainBuffet, - "invalid_parameter", "Invalid name: %s", - name.c_str()); + BuffetConfig::Transaction change{config_.get()}; + if (!change.set_name(name)) { + chromeos::Error::AddTo(error, FROM_HERE, kErrorDomainBuffet, + "invalid_parameter", "Empty device name"); return false; } - config_->set_description(description); - config_->set_location(location); - - Save(); - OnConfigChanged(); + change.set_description(description); + change.set_location(location); if (HaveRegistrationCredentials(nullptr)) { UpdateDeviceResource(base::Bind(&base::DoNothing), @@ -878,10 +814,8 @@ const CloudRequestErrorCallback& on_failure) { DoCloudRequest( chromeos::http::request_type::kGet, - GetServiceURL("commands/queue", {{"deviceId", device_id_}}), - nullptr, - base::Bind(&HandleFetchCommandsResult, on_success), - on_failure); + GetServiceURL("commands/queue", {{"deviceId", config_->device_id()}}), + nullptr, base::Bind(&HandleFetchCommandsResult, on_success), on_failure); } void DeviceRegistrationInfo::AbortLimboCommands( @@ -1021,29 +955,11 @@ void DeviceRegistrationInfo::SetRegistrationStatus( RegistrationStatus new_status) { - registration_status_ = new_status; - if (manager_) - manager_->SetStatus(StatusToString(registration_status_)); VLOG_IF(1, new_status != registration_status_) << "Changing registration status to " << StatusToString(new_status); -} - -void DeviceRegistrationInfo::SetDeviceId(const std::string& device_id) { - device_id_ = device_id; - if (manager_) - manager_->SetDeviceId(device_id_); -} - -void DeviceRegistrationInfo::OnConfigChanged() { - if (!manager_) - return; - manager_->SetOemName(config_->oem_name()); - manager_->SetModelName(config_->model_name()); - manager_->SetModelId(config_->model_id()); - manager_->SetName(config_->name()); - manager_->SetDescription(config_->description()); - manager_->SetLocation(config_->location()); - manager_->SetAnonymousAccessRole(config_->local_anonymous_access_role()); + registration_status_ = new_status; + for (const auto& cb : on_registration_changed_) + cb.Run(registration_status_); } void DeviceRegistrationInfo::OnCommandDefsChanged() {
diff --git a/buffet/device_registration_info.h b/buffet/device_registration_info.h index ab72c87..29c73d2 100644 --- a/buffet/device_registration_info.h +++ b/buffet/device_registration_info.h
@@ -9,6 +9,7 @@ #include <memory> #include <string> #include <utility> +#include <vector> #include <base/macros.h> #include <base/memory/weak_ptr.h> @@ -25,14 +26,6 @@ #include "buffet/registration_status.h" #include "buffet/storage_interface.h" -namespace org { -namespace chromium { -namespace Buffet { -class ManagerAdaptor; -} -} -} - namespace base { class DictionaryValue; } // namespace base @@ -52,24 +45,21 @@ // The DeviceRegistrationInfo class represents device registration information. class DeviceRegistrationInfo : public NotificationDelegate { public: - // This is a helper class for unit testing. - class TestHelper; + using OnRegistrationChangedCallback = + base::Callback<void(RegistrationStatus)>; DeviceRegistrationInfo( const std::shared_ptr<CommandManager>& command_manager, const std::shared_ptr<StateManager>& state_manager, std::unique_ptr<BuffetConfig> config, const std::shared_ptr<chromeos::http::Transport>& transport, - const std::shared_ptr<StorageInterface>& state_store, - bool notifications_enabled, - org::chromium::Buffet::ManagerAdaptor* manager); + bool notifications_enabled); virtual ~DeviceRegistrationInfo(); - // Returns our current best known registration status. - RegistrationStatus GetRegistrationStatus() const { - return registration_status_; - } + // Add callback to listen for changes in registration status. + void AddOnRegistrationChangedCallback( + const OnRegistrationChangedCallback& callback); // Returns the authorization HTTP header that can be used to talk // to GCD server for authenticated device communication. @@ -103,11 +93,8 @@ const std::string& subpath = {}, const chromeos::data_encoding::WebParamList& params = {}) const; - // Returns the registered device ID (GUID) or empty string. - const std::string& GetDeviceId() const; - - // Loads the device registration information from cache. - bool Load(); + // Starts GCD device if credentials avalible. + void Start(); // Checks whether we have credentials generated during registration. bool HaveRegistrationCredentials(chromeos::ErrorPtr* error); @@ -124,9 +111,8 @@ // is omitted, a default value is used when possible. // Returns a device ID on success. // The values are all strings for now. - std::string RegisterDevice( - const std::map<std::string, std::string>& params, - chromeos::ErrorPtr* error); + std::string RegisterDevice(const std::map<std::string, std::string>& params, + chromeos::ErrorPtr* error); // Updates a command. void UpdateCommand(const std::string& command_id, @@ -147,6 +133,8 @@ } private: + friend class DeviceRegistrationInfoTest; + // Cause DeviceRegistrationInfo to attempt to StartDevice on its own later. void ScheduleStartDevice(const base::TimeDelta& later); @@ -157,9 +145,6 @@ void StartDevice(chromeos::ErrorPtr* error, const base::TimeDelta& retry_delay); - // Saves the device registration to cache. - bool Save() const; - // Checks for the valid device registration as well as refreshes // the device access token, if available. bool CheckRegistration(chromeos::ErrorPtr* error); @@ -232,7 +217,6 @@ void SetRegistrationStatus(RegistrationStatus new_status); void SetDeviceId(const std::string& device_id); - void OnConfigChanged(); // Callback called when command definitions are changed to re-publish new CDD. void OnCommandDefsChanged(); @@ -242,19 +226,12 @@ void OnDisconnected() override; void OnPermanentFailure() override; - // Data that is cached here, persisted in the state store. - std::string refresh_token_; - std::string device_id_; - std::string device_robot_account_; - // Transient data std::string access_token_; base::Time access_token_expiration_; // HTTP transport used for communications. std::shared_ptr<chromeos::http::Transport> transport_; - // Serialization interface to save and load device registration info. - std::shared_ptr<StorageInterface> storage_; // Global command manager. std::shared_ptr<CommandManager> command_manager_; // Device state manager. @@ -267,12 +244,12 @@ // Tracks our current registration status. RegistrationStatus registration_status_{RegistrationStatus::kUnconfigured}; - org::chromium::Buffet::ManagerAdaptor* manager_; base::RepeatingTimer<DeviceRegistrationInfo> command_poll_timer_; base::RepeatingTimer<DeviceRegistrationInfo> state_push_timer_; - friend class TestHelper; + std::vector<OnRegistrationChangedCallback> on_registration_changed_; + base::WeakPtrFactory<DeviceRegistrationInfo> weak_factory_{this}; DISALLOW_COPY_AND_ASSIGN(DeviceRegistrationInfo); };
diff --git a/buffet/device_registration_info_unittest.cc b/buffet/device_registration_info_unittest.cc index 934ae51..f41d408 100644 --- a/buffet/device_registration_info_unittest.cc +++ b/buffet/device_registration_info_unittest.cc
@@ -162,34 +162,26 @@ } // anonymous namespace -// This is a helper class that allows the unit tests to access private -// methods and data since TestHelper is declared as a friend to -// DeviceRegistrationInfo. -class DeviceRegistrationInfo::TestHelper { - public: - static bool Save(DeviceRegistrationInfo* info) { - return info->Save(); - } - - static void PublishCommands(DeviceRegistrationInfo* info, - const base::ListValue& commands) { - return info->PublishCommands(commands); - } - - static bool CheckRegistration(DeviceRegistrationInfo* info, - chromeos::ErrorPtr* error) { - return info->CheckRegistration(error); - } -}; - class DeviceRegistrationInfoTest : public ::testing::Test { protected: void SetUp() override { - storage_ = std::make_shared<MemStorage>(); - storage_->Save(data_); + std::unique_ptr<StorageInterface> storage{new MemStorage}; + storage_ = storage.get(); + storage->Save(data_); transport_ = std::make_shared<chromeos::http::fake::Transport>(); command_manager_ = std::make_shared<CommandManager>(); state_manager_ = std::make_shared<StateManager>(&mock_state_change_queue_); + + std::unique_ptr<BuffetConfig> config{new BuffetConfig{std::move(storage)}}; + config_ = config.get(); + dev_reg_ = std::unique_ptr<DeviceRegistrationInfo>( + new DeviceRegistrationInfo(command_manager_, state_manager_, + std::move(config), transport_, true)); + + ReloadConfig(); + } + + void ReloadConfig() { chromeos::KeyValueStore config_store; config_store.SetString("client_id", test_data::kClientId); config_store.SetString("client_secret", test_data::kClientSecret); @@ -199,23 +191,28 @@ config_store.SetString("description", "Easy to clean"); config_store.SetString("location", "Kitchen"); config_store.SetString("local_anonymous_access_role", "viewer"); - config_store.SetBoolean("local_local_discovery_enabled", true); - config_store.SetBoolean("local_local_pairing_enabled", false); config_store.SetString("model_id", "AAAAA"); config_store.SetString("oauth_url", test_data::kOAuthURL); config_store.SetString("service_url", test_data::kServiceURL); - std::unique_ptr<BuffetConfig> config{new BuffetConfig}; - config->Load(config_store); - dev_reg_ = std::unique_ptr<DeviceRegistrationInfo>( - new DeviceRegistrationInfo(command_manager_, state_manager_, - std::move(config), - transport_, storage_, - true, - nullptr)); + config_->Load(config_store); + dev_reg_->Start(); + } + + void PublishCommands(const base::ListValue& commands) { + return dev_reg_->PublishCommands(commands); + } + + bool CheckRegistration(chromeos::ErrorPtr* error) const { + return dev_reg_->CheckRegistration(error); + } + + RegistrationStatus GetRegistrationStatus() const { + return dev_reg_->registration_status_; } base::DictionaryValue data_; - std::shared_ptr<MemStorage> storage_; + StorageInterface* storage_{nullptr}; + BuffetConfig* config_{nullptr}; std::shared_ptr<chromeos::http::fake::Transport> transport_; std::unique_ptr<DeviceRegistrationInfo> dev_reg_; std::shared_ptr<CommandManager> command_manager_; @@ -225,7 +222,6 @@ //////////////////////////////////////////////////////////////////////////////// TEST_F(DeviceRegistrationInfoTest, GetServiceURL) { - EXPECT_TRUE(dev_reg_->Load()); EXPECT_EQ(test_data::kServiceURL, dev_reg_->GetServiceURL()); std::string url = test_data::kServiceURL; url += "registrationTickets"; @@ -242,35 +238,7 @@ })); } -TEST_F(DeviceRegistrationInfoTest, VerifySave) { - auto expected = R"({ - 'description': 'l', - 'device_id': 'e', - 'local_anonymous_access_role': 'user', - 'local_discovery_enabled': true, - 'local_pairing_enabled': true, - 'location': 'm', - 'name': 'k', - 'refresh_token': 'd', - 'robot_account': 'h' - })"; - - storage_->Save(*unittests::CreateDictionaryValue(expected)); - - // This test isn't really trying to test Load, it is just the easiest - // way to initialize the properties in dev_reg_. - EXPECT_TRUE(dev_reg_->Load()); - - // Clear the storage to get a clean test. - base::DictionaryValue empty; - storage_->Save(empty); - EXPECT_TRUE(DeviceRegistrationInfo::TestHelper::Save(dev_reg_.get())); - - EXPECT_JSON_EQ(expected, *storage_->Load()); -} - TEST_F(DeviceRegistrationInfoTest, GetOAuthURL) { - EXPECT_TRUE(dev_reg_->Load()); EXPECT_EQ(test_data::kOAuthURL, dev_reg_->GetOAuthURL()); std::string url = test_data::kOAuthURL; url += "auth?scope=https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fclouddevices&"; @@ -287,65 +255,59 @@ } TEST_F(DeviceRegistrationInfoTest, CheckRegistration) { - EXPECT_TRUE(dev_reg_->Load()); - EXPECT_FALSE(DeviceRegistrationInfo::TestHelper::CheckRegistration( - dev_reg_.get(), nullptr)); + EXPECT_FALSE(CheckRegistration(nullptr)); EXPECT_EQ(0, transport_->GetRequestCount()); SetDefaultDeviceRegistration(&data_); storage_->Save(data_); - EXPECT_TRUE(dev_reg_->Load()); + ReloadConfig(); transport_->AddHandler(dev_reg_->GetOAuthURL("token"), chromeos::http::request_type::kPost, base::Bind(OAuth2Handler)); transport_->ResetRequestCount(); - EXPECT_TRUE(DeviceRegistrationInfo::TestHelper::CheckRegistration( - dev_reg_.get(), nullptr)); + EXPECT_TRUE(CheckRegistration(nullptr)); EXPECT_EQ(1, transport_->GetRequestCount()); } TEST_F(DeviceRegistrationInfoTest, CheckAuthenticationFailure) { SetDefaultDeviceRegistration(&data_); storage_->Save(data_); - EXPECT_TRUE(dev_reg_->Load()); - EXPECT_EQ(RegistrationStatus::kConnecting, dev_reg_->GetRegistrationStatus()); + ReloadConfig(); + EXPECT_EQ(RegistrationStatus::kConnecting, GetRegistrationStatus()); transport_->AddHandler(dev_reg_->GetOAuthURL("token"), chromeos::http::request_type::kPost, base::Bind(OAuth2HandlerFail)); transport_->ResetRequestCount(); chromeos::ErrorPtr error; - EXPECT_FALSE(DeviceRegistrationInfo::TestHelper::CheckRegistration( - dev_reg_.get(), &error)); + EXPECT_FALSE(CheckRegistration(&error)); EXPECT_EQ(1, transport_->GetRequestCount()); EXPECT_TRUE(error->HasError(kErrorDomainOAuth2, "unable_to_authenticate")); - EXPECT_EQ(RegistrationStatus::kConnecting, dev_reg_->GetRegistrationStatus()); + EXPECT_EQ(RegistrationStatus::kConnecting, GetRegistrationStatus()); } TEST_F(DeviceRegistrationInfoTest, CheckDeregistration) { SetDefaultDeviceRegistration(&data_); storage_->Save(data_); - EXPECT_TRUE(dev_reg_->Load()); - EXPECT_EQ(RegistrationStatus::kConnecting, dev_reg_->GetRegistrationStatus()); + ReloadConfig(); + EXPECT_EQ(RegistrationStatus::kConnecting, GetRegistrationStatus()); transport_->AddHandler(dev_reg_->GetOAuthURL("token"), chromeos::http::request_type::kPost, base::Bind(OAuth2HandlerDeregister)); transport_->ResetRequestCount(); chromeos::ErrorPtr error; - EXPECT_FALSE(DeviceRegistrationInfo::TestHelper::CheckRegistration( - dev_reg_.get(), &error)); + EXPECT_FALSE(CheckRegistration(&error)); EXPECT_EQ(1, transport_->GetRequestCount()); EXPECT_TRUE(error->HasError(kErrorDomainOAuth2, "invalid_grant")); - EXPECT_EQ(RegistrationStatus::kInvalidCredentials, - dev_reg_->GetRegistrationStatus()); + EXPECT_EQ(RegistrationStatus::kInvalidCredentials, GetRegistrationStatus()); } TEST_F(DeviceRegistrationInfoTest, GetDeviceInfo) { SetDefaultDeviceRegistration(&data_); storage_->Save(data_); - EXPECT_TRUE(dev_reg_->Load()); + ReloadConfig(); transport_->AddHandler(dev_reg_->GetOAuthURL("token"), chromeos::http::request_type::kPost, @@ -367,7 +329,7 @@ TEST_F(DeviceRegistrationInfoTest, GetDeviceId) { SetDefaultDeviceRegistration(&data_); storage_->Save(data_); - EXPECT_TRUE(dev_reg_->Load()); + ReloadConfig(); transport_->AddHandler(dev_reg_->GetOAuthURL("token"), chromeos::http::request_type::kPost, @@ -375,12 +337,10 @@ transport_->AddHandler(dev_reg_->GetDeviceURL(), chromeos::http::request_type::kGet, base::Bind(DeviceInfoHandler)); - EXPECT_EQ(test_data::kDeviceId, dev_reg_->GetDeviceId()); + EXPECT_EQ(test_data::kDeviceId, config_->device_id()); } TEST_F(DeviceRegistrationInfoTest, RegisterDevice) { - EXPECT_TRUE(dev_reg_->Load()); - auto update_ticket = [](const ServerRequest& request, ServerResponse* response) { EXPECT_EQ(test_data::kApiKey, request.GetFormField("key")); @@ -487,16 +447,13 @@ transport_->AddHandler(dev_reg_->GetOAuthURL("token"), chromeos::http::request_type::kPost, base::Bind(OAuth2Handler)); - storage_->reset_save_count(); - std::map<std::string, std::string> params; params["ticket_id"] = test_data::kClaimTicketId; std::string device_id = dev_reg_->RegisterDevice(params, nullptr); EXPECT_EQ(test_data::kDeviceId, device_id); - EXPECT_GT(storage_->save_count(), 1); // The state must have been saved. EXPECT_EQ(3, transport_->GetRequestCount()); - EXPECT_EQ(RegistrationStatus::kConnecting, dev_reg_->GetRegistrationStatus()); + EXPECT_EQ(RegistrationStatus::kConnecting, GetRegistrationStatus()); // Validate the device info saved to storage... auto storage_data = storage_->Load(); @@ -514,18 +471,15 @@ TEST_F(DeviceRegistrationInfoTest, OOBRegistrationStatus) { // After we've been initialized, we should be either offline or unregistered, // depending on whether or not we've found credentials. - EXPECT_TRUE(dev_reg_->Load()); - EXPECT_EQ(RegistrationStatus::kUnconfigured, - dev_reg_->GetRegistrationStatus()); + EXPECT_EQ(RegistrationStatus::kUnconfigured, GetRegistrationStatus()); // Put some credentials into our state, make sure we call that offline. SetDefaultDeviceRegistration(&data_); storage_->Save(data_); - EXPECT_TRUE(dev_reg_->Load()); - EXPECT_EQ(RegistrationStatus::kConnecting, dev_reg_->GetRegistrationStatus()); + ReloadConfig(); + EXPECT_EQ(RegistrationStatus::kConnecting, GetRegistrationStatus()); } TEST_F(DeviceRegistrationInfoTest, UpdateCommand) { - EXPECT_TRUE(dev_reg_->Load()); auto json_cmds = unittests::CreateDictionaryValue(R"({ 'robot': { '_jump': { @@ -546,8 +500,7 @@ ASSERT_NE(nullptr, commands_json.get()); const base::ListValue* command_list = nullptr; ASSERT_TRUE(commands_json->GetAsList(&command_list)); - DeviceRegistrationInfo::TestHelper::PublishCommands(dev_reg_.get(), - *command_list); + PublishCommands(*command_list); auto command = command_manager_->FindCommand("1234"); ASSERT_NE(nullptr, command); StringPropType string_type;
diff --git a/buffet/main.cc b/buffet/main.cc index 7d70ffd..a4ecf59 100644 --- a/buffet/main.cc +++ b/buffet/main.cc
@@ -42,7 +42,6 @@ } private: - BuffetConfig config_; std::unique_ptr<buffet::Manager> manager_; const base::FilePath config_path_; const base::FilePath state_path_;
diff --git a/buffet/manager.cc b/buffet/manager.cc index 90562aa..2002eb2 100644 --- a/buffet/manager.cc +++ b/buffet/manager.cc
@@ -61,20 +61,24 @@ state_change_queue_.reset(new StateChangeQueue(kMaxStateChangeQueueSize)); state_manager_ = std::make_shared<StateManager>(state_change_queue_.get()); state_manager_->Startup(); - std::unique_ptr<BuffetConfig> config{new BuffetConfig}; + + std::unique_ptr<BuffetConfig> config{new BuffetConfig{state_path}}; + config->AddOnChangedCallback( + base::Bind(&Manager::OnConfigChanged, weak_ptr_factory_.GetWeakPtr())); config->Load(config_path); - std::unique_ptr<FileStorage> state_store{new FileStorage{state_path}}; + // TODO(avakulenko): Figure out security implications of storing // device info state data unencrypted. device_info_.reset(new DeviceRegistrationInfo( command_manager_, state_manager_, std::move(config), - chromeos::http::Transport::CreateDefault(), std::move(state_store), - xmpp_enabled, &dbus_adaptor_)); + chromeos::http::Transport::CreateDefault(), xmpp_enabled)); + device_info_->AddOnRegistrationChangedCallback(base::Bind( + &Manager::OnRegistrationChanged, weak_ptr_factory_.GetWeakPtr())); base_api_handler_.reset( new BaseApiHandler{device_info_->AsWeakPtr(), command_manager_}); - device_info_->Load(); + device_info_->Start(); dbus_adaptor_.RegisterWithDBusObject(&dbus_object_); dbus_object_.RegisterAsync(cb); } @@ -91,7 +95,8 @@ return; } - response->Return(registered ? device_info_->GetDeviceId() : std::string()); + response->Return(registered ? device_info_->GetConfig().device_id() + : std::string()); } void Manager::GetDeviceInfo(DBusMethodResponse<std::string> response) { @@ -252,4 +257,19 @@ dbus_adaptor_.SetCommandDefs(json); } +void Manager::OnRegistrationChanged(RegistrationStatus status) { + dbus_adaptor_.SetStatus(StatusToString(status)); +} + +void Manager::OnConfigChanged(const BuffetConfig& config) { + dbus_adaptor_.SetDeviceId(config.device_id()); + dbus_adaptor_.SetOemName(config.oem_name()); + dbus_adaptor_.SetModelName(config.model_name()); + dbus_adaptor_.SetModelId(config.model_id()); + dbus_adaptor_.SetName(config.name()); + dbus_adaptor_.SetDescription(config.description()); + dbus_adaptor_.SetLocation(config.location()); + dbus_adaptor_.SetAnonymousAccessRole(config.local_anonymous_access_role()); +} + } // namespace buffet
diff --git a/buffet/manager.h b/buffet/manager.h index 44790d3..3961b68 100644 --- a/buffet/manager.h +++ b/buffet/manager.h
@@ -81,6 +81,8 @@ std::string TestMethod(const std::string& message) override; void OnCommandDefsChanged(); + void OnRegistrationChanged(RegistrationStatus status); + void OnConfigChanged(const BuffetConfig& config); org::chromium::Buffet::ManagerAdaptor dbus_adaptor_{this}; chromeos::dbus_utils::DBusObject dbus_object_;
diff --git a/buffet/storage_impls.cc b/buffet/storage_impls.cc index ea29d69..b0bcfd9 100644 --- a/buffet/storage_impls.cc +++ b/buffet/storage_impls.cc
@@ -39,7 +39,6 @@ bool MemStorage::Save(const base::DictionaryValue& config) { cache_.Clear(); cache_.MergeDictionary(&config); - ++save_count_; return true; }
diff --git a/buffet/storage_impls.h b/buffet/storage_impls.h index bacedd0..7f205da 100644 --- a/buffet/storage_impls.h +++ b/buffet/storage_impls.h
@@ -33,11 +33,8 @@ virtual ~MemStorage() = default; std::unique_ptr<base::DictionaryValue> Load() override; bool Save(const base::DictionaryValue& config) override; - int save_count() { return save_count_; } - void reset_save_count() { save_count_ = 0; } private: - int save_count_ = 0; base::DictionaryValue cache_; DISALLOW_COPY_AND_ASSIGN(MemStorage); };