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);
};
