buffet: Remove initial OnRegistrationStatusChanged call
This call was neccecary because DeviceRegistrationInfo calls callback only
if values actually changed, skipping callback for intial value. We still need to
call callback there too. Solved by additional stus change in Load().
Also some reformating and renames.
BUG=brillo:592
TEST=covered by buffet_Registration
Change-Id: Iafa8e5b1eac342a99c54c3c067a742182221186f
Reviewed-on: https://chromium-review.googlesource.com/261589
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/device_registration_info.cc b/buffet/device_registration_info.cc
index f14ca21..380d75a 100644
--- a/buffet/device_registration_info.cc
+++ b/buffet/device_registration_info.cc
@@ -136,13 +136,13 @@
std::unique_ptr<chromeos::KeyValueStore> config_store,
const std::shared_ptr<chromeos::http::Transport>& transport,
const std::shared_ptr<StorageInterface>& state_store,
- const StatusHandler& status_handler)
+ const base::Closure& on_status_changed)
: transport_{transport},
storage_{state_store},
command_manager_{command_manager},
state_manager_{state_manager},
config_store_{std::move(config_store)},
- registration_status_handler_{status_handler} {
+ on_status_changed_{on_status_changed} {
}
DeviceRegistrationInfo::~DeviceRegistrationInfo() = default;
@@ -176,7 +176,10 @@
}
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))
@@ -996,16 +999,16 @@
return;
VLOG(1) << "Changing registration status to " << StatusToString(new_status);
registration_status_ = new_status;
- if (!registration_status_handler_.is_null())
- registration_status_handler_.Run();
+ if (!on_status_changed_.is_null())
+ on_status_changed_.Run();
}
void DeviceRegistrationInfo::SetDeviceId(const std::string& device_id) {
if (device_id == device_id_)
return;
device_id_ = device_id;
- if (!registration_status_handler_.is_null())
- registration_status_handler_.Run();
+ if (!on_status_changed_.is_null())
+ on_status_changed_.Run();
}
} // namespace buffet
diff --git a/buffet/device_registration_info.h b/buffet/device_registration_info.h
index c519048..f985d42 100644
--- a/buffet/device_registration_info.h
+++ b/buffet/device_registration_info.h
@@ -45,7 +45,6 @@
public:
// This is a helper class for unit testing.
class TestHelper;
- using StatusHandler = base::Closure;
DeviceRegistrationInfo(
const std::shared_ptr<CommandManager>& command_manager,
@@ -53,7 +52,7 @@
std::unique_ptr<chromeos::KeyValueStore> config_store,
const std::shared_ptr<chromeos::http::Transport>& transport,
const std::shared_ptr<StorageInterface>& state_store,
- const StatusHandler& status_handler);
+ const base::Closure& on_status_changed);
~DeviceRegistrationInfo() override;
@@ -256,7 +255,7 @@
// Tracks our current registration status.
RegistrationStatus registration_status_{RegistrationStatus::kUnconfigured};
- StatusHandler registration_status_handler_;
+ base::Closure on_status_changed_;
friend class TestHelper;
base::WeakPtrFactory<DeviceRegistrationInfo> weak_factory_{this};
diff --git a/buffet/manager.cc b/buffet/manager.cc
index 53ec9c9..0855384 100644
--- a/buffet/manager.cc
+++ b/buffet/manager.cc
@@ -39,15 +39,16 @@
Manager::Manager(const base::WeakPtr<ExportedObjectManager>& object_manager)
: dbus_object_(object_manager.get(),
object_manager->GetBus(),
- org::chromium::Buffet::ManagerAdaptor::GetObjectPath()) {}
+ org::chromium::Buffet::ManagerAdaptor::GetObjectPath()) {
+}
-Manager::~Manager() {}
+Manager::~Manager() {
+}
-void Manager::RegisterAsync(
- const base::FilePath& config_path,
- const base::FilePath& state_path,
- const base::FilePath& test_definitions_path,
- const AsyncEventSequencer::CompletionAction& cb) {
+void Manager::RegisterAsync(const base::FilePath& config_path,
+ const base::FilePath& state_path,
+ const base::FilePath& test_definitions_path,
+ const AsyncEventSequencer::CompletionAction& cb) {
command_manager_ =
std::make_shared<CommandManager>(dbus_object_.GetObjectManager());
command_manager_->Startup(base::FilePath{"/etc/buffet"},
@@ -69,10 +70,8 @@
std::move(config_store),
chromeos::http::Transport::CreateDefault(),
std::move(state_store),
- base::Bind(&Manager::OnRegistrationStatusChange,
+ base::Bind(&Manager::OnRegistrationStatusChanged,
base::Unretained(this))));
- // Reset D-Bus properties.
- OnRegistrationStatusChange();
device_info_->Load();
dbus_adaptor_.RegisterWithDBusObject(&dbus_object_);
dbus_object_.RegisterAsync(cb);
@@ -121,8 +120,8 @@
"String value expected");
return;
}
- str_params.emplace_hint(str_params.end(),
- pair.first, pair.second.Get<std::string>());
+ str_params.emplace_hint(str_params.end(), pair.first,
+ pair.second.Get<std::string>());
}
std::string device_id = device_info_->RegisterDevice(str_params, &error);
if (!device_id.empty()) {
@@ -132,8 +131,7 @@
if (!error) {
// TODO(zeuthen): This can be changed to CHECK(error) once
// RegisterDevice() has been fixed to set |error| when failing.
- chromeos::Error::AddTo(&error, FROM_HERE, kErrorDomainGCD,
- "internal_error",
+ chromeos::Error::AddTo(&error, FROM_HERE, kErrorDomainGCD, "internal_error",
"device_id empty but error not set");
}
response->ReplyWithError(error.get());
@@ -145,8 +143,8 @@
base::Time timestamp = base::Time::Now();
bool all_success = true;
for (const auto& pair : property_set) {
- if (!state_manager_->SetPropertyValue(pair.first, pair.second,
- timestamp, &error)) {
+ if (!state_manager_->SetPropertyValue(pair.first, pair.second, timestamp,
+ &error)) {
// Remember that an error occurred but keep going and update the rest of
// the properties if possible.
all_success = false;
@@ -195,7 +193,7 @@
return message;
}
-void Manager::OnRegistrationStatusChange() {
+void Manager::OnRegistrationStatusChanged() {
dbus_adaptor_.SetStatus(
StatusToString(device_info_->GetRegistrationStatus()));
dbus_adaptor_.SetDeviceId(device_info_->GetDeviceId());
diff --git a/buffet/manager.h b/buffet/manager.h
index da3c044..0136019 100644
--- a/buffet/manager.h
+++ b/buffet/manager.h
@@ -73,7 +73,7 @@
// Handles calls to org.chromium.Buffet.Manager.Test()
std::string TestMethod(const std::string& message) override;
- void OnRegistrationStatusChange();
+ void OnRegistrationStatusChanged();
org::chromium::Buffet::ManagerAdaptor dbus_adaptor_{this};
chromeos::dbus_utils::DBusObject dbus_object_;
