buffet: Add callback to notify about state changes
Callback could be used to expose state as D-Bus property or
for smarter server updates.
SetProperties replaced SetPropertyValue to avoid multiple callbacks
for complex state updates.
BUG=brillo:1124,brillo:810,brillo:427
TEST=`FEATURES=test emerge-gizmo buffet`
Change-Id: I899b14fd41b0a45cabe149431a3bebeba9c50320
Reviewed-on: https://chromium-review.googlesource.com/273341
Tested-by: Vitaly Buka <vitalybuka@chromium.org>
Trybot-Ready: Vitaly Buka <vitalybuka@chromium.org>
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
diff --git a/buffet/manager.cc b/buffet/manager.cc
index 78a0a8c..96319c0 100644
--- a/buffet/manager.cc
+++ b/buffet/manager.cc
@@ -137,17 +137,7 @@
void Manager::UpdateState(DBusMethodResponse<> response,
const chromeos::VariantDictionary& property_set) {
chromeos::ErrorPtr error;
- 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)) {
- // Remember that an error occurred but keep going and update the rest of
- // the properties if possible.
- all_success = false;
- }
- }
- if (!all_success)
+ if (!state_manager_->SetProperties(property_set, &error))
response->ReplyWithError(error.get());
else
response->Return();
diff --git a/buffet/states/state_manager.cc b/buffet/states/state_manager.cc
index 827cd7f..57330f4 100644
--- a/buffet/states/state_manager.cc
+++ b/buffet/states/state_manager.cc
@@ -30,6 +30,11 @@
CHECK(state_change_queue_) << "State change queue not specified";
}
+void StateManager::AddOnChangedCallback(const base::Closure& callback) {
+ on_changed_.push_back(callback);
+ callback.Run(); // Force to read current state.
+}
+
void StateManager::Startup() {
LOG(INFO) << "Initializing StateManager.";
@@ -88,6 +93,9 @@
firmware_version,
base::Time::Now(),
nullptr));
+
+ for (const auto& cb : on_changed_)
+ cb.Run();
}
std::unique_ptr<base::DictionaryValue> StateManager::GetStateValuesAsJson(
@@ -104,6 +112,23 @@
return dict;
}
+bool StateManager::SetProperties(
+ const chromeos::VariantDictionary& property_set,
+ chromeos::ErrorPtr* error) {
+ base::Time timestamp = base::Time::Now();
+ bool all_success = true;
+ for (const auto& pair : property_set) {
+ if (!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;
+ }
+ }
+ for (const auto& cb : on_changed_)
+ cb.Run();
+ return all_success;
+}
+
bool StateManager::SetPropertyValue(const std::string& full_property_name,
const chromeos::Any& value,
const base::Time& timestamp,
diff --git a/buffet/states/state_manager.h b/buffet/states/state_manager.h
index 53695e2..cf2c60a 100644
--- a/buffet/states/state_manager.h
+++ b/buffet/states/state_manager.h
@@ -11,6 +11,7 @@
#include <string>
#include <vector>
+#include <base/callback.h>
#include <base/macros.h>
#include <chromeos/errors/error.h>
#include <chromeos/variant_dictionary.h>
@@ -33,6 +34,8 @@
public:
explicit StateManager(StateChangeQueueInterface* state_change_queue);
+ void AddOnChangedCallback(const base::Closure& callback);
+
// Initializes the state manager and load device state fragments.
// Called by Buffet daemon at startup.
void Startup();
@@ -42,12 +45,9 @@
std::unique_ptr<base::DictionaryValue> GetStateValuesAsJson(
chromeos::ErrorPtr* error) const;
- // Updates a single property value. |full_property_name| must be the full
- // name of the property to update in format "package.property".
- bool SetPropertyValue(const std::string& full_property_name,
- const chromeos::Any& value,
- const base::Time& timestamp,
- chromeos::ErrorPtr* error);
+ // Updates a multiple property values.
+ bool SetProperties(const chromeos::VariantDictionary& property_set,
+ chromeos::ErrorPtr* error);
// Returns all the categories the state properties are registered from.
// As with GCD command handling, the category normally represent a device
@@ -61,6 +61,15 @@
std::vector<StateChange> GetAndClearRecordedStateChanges();
private:
+ friend class StateManagerTest;
+
+ // Updates a single property value. |full_property_name| must be the full
+ // name of the property to update in format "package.property".
+ bool SetPropertyValue(const std::string& full_property_name,
+ const chromeos::Any& value,
+ const base::Time& timestamp,
+ chromeos::ErrorPtr* error);
+
// Loads a device state fragment from a JSON object. |category| represents
// a device daemon providing the state fragment or empty string for the
// base state fragment.
@@ -96,7 +105,8 @@
std::map<std::string, std::unique_ptr<StatePackage>> packages_;
std::set<std::string> categories_;
- friend class StateManagerTest;
+ std::vector<base::Closure> on_changed_;
+
DISALLOW_COPY_AND_ASSIGN(StateManager);
};
diff --git a/buffet/states/state_manager_unittest.cc b/buffet/states/state_manager_unittest.cc
index d71ebb6..50e6a5f 100644
--- a/buffet/states/state_manager_unittest.cc
+++ b/buffet/states/state_manager_unittest.cc
@@ -7,6 +7,7 @@
#include <cstdlib> // for abs().
#include <vector>
+#include <base/bind.h>
#include <base/values.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
@@ -56,6 +57,11 @@
EXPECT_CALL(mock_state_change_queue_, GetAndClearRecordedStateChanges())
.Times(0);
mgr_.reset(new StateManager(&mock_state_change_queue_));
+
+ EXPECT_CALL(*this, OnStateChanged()).Times(1);
+ mgr_->AddOnChangedCallback(
+ base::Bind(&StateManagerTest::OnStateChanged, base::Unretained(this)));
+
LoadStateDefinition(GetTestSchema().get(), "default", nullptr);
ASSERT_TRUE(mgr_->LoadStateDefaults(*GetTestValues().get(), nullptr));
}
@@ -67,6 +73,15 @@
ASSERT_TRUE(mgr_->LoadStateDefinition(*json, category, error));
}
+ bool SetPropertyValue(const std::string& name,
+ const chromeos::Any& value,
+ chromeos::ErrorPtr* error) {
+ return mgr_->SetPropertyValue(name, value, timestamp_, error);
+ }
+
+ MOCK_CONST_METHOD0(OnStateChanged, void());
+
+ base::Time timestamp_{base::Time::Now()};
std::unique_ptr<StateManager> mgr_;
MockStateChangeQueueInterface mock_state_change_queue_;
};
@@ -120,12 +135,11 @@
native_types::Object expected_prop_set{
{"terminator.target", unittests::make_string_prop_value("John Connor")},
};
- base::Time timestamp = base::Time::Now();
EXPECT_CALL(mock_state_change_queue_,
- NotifyPropertiesUpdated(timestamp, expected_prop_set))
+ NotifyPropertiesUpdated(timestamp_, expected_prop_set))
.WillOnce(Return(true));
- ASSERT_TRUE(mgr_->SetPropertyValue(
- "terminator.target", std::string{"John Connor"}, timestamp, nullptr));
+ ASSERT_TRUE(SetPropertyValue("terminator.target", std::string{"John Connor"},
+ nullptr));
auto expected = R"({
'base': {
'manufacturer': 'Skynet',
@@ -140,7 +154,7 @@
TEST_F(StateManagerTest, SetPropertyValue_Error_NoName) {
chromeos::ErrorPtr error;
- ASSERT_FALSE(mgr_->SetPropertyValue("", int{0}, base::Time::Now(), &error));
+ ASSERT_FALSE(SetPropertyValue("", int{0}, &error));
EXPECT_EQ(errors::state::kDomain, error->GetDomain());
EXPECT_EQ(errors::state::kPropertyNameMissing, error->GetCode());
EXPECT_EQ("Property name is missing", error->GetMessage());
@@ -148,8 +162,7 @@
TEST_F(StateManagerTest, SetPropertyValue_Error_NoPackage) {
chromeos::ErrorPtr error;
- ASSERT_FALSE(
- mgr_->SetPropertyValue("target", int{0}, base::Time::Now(), &error));
+ ASSERT_FALSE(SetPropertyValue("target", int{0}, &error));
EXPECT_EQ(errors::state::kDomain, error->GetDomain());
EXPECT_EQ(errors::state::kPackageNameMissing, error->GetCode());
EXPECT_EQ("Package name is missing in the property name",
@@ -158,8 +171,7 @@
TEST_F(StateManagerTest, SetPropertyValue_Error_UnknownPackage) {
chromeos::ErrorPtr error;
- ASSERT_FALSE(
- mgr_->SetPropertyValue("power.level", int{0}, base::Time::Now(), &error));
+ ASSERT_FALSE(SetPropertyValue("power.level", int{0}, &error));
EXPECT_EQ(errors::state::kDomain, error->GetDomain());
EXPECT_EQ(errors::state::kPropertyNotDefined, error->GetCode());
EXPECT_EQ("Unknown state property package 'power'", error->GetMessage());
@@ -167,22 +179,20 @@
TEST_F(StateManagerTest, SetPropertyValue_Error_UnknownProperty) {
chromeos::ErrorPtr error;
- ASSERT_FALSE(
- mgr_->SetPropertyValue("base.level", int{0}, base::Time::Now(), &error));
+ ASSERT_FALSE(SetPropertyValue("base.level", int{0}, &error));
EXPECT_EQ(errors::state::kDomain, error->GetDomain());
EXPECT_EQ(errors::state::kPropertyNotDefined, error->GetCode());
EXPECT_EQ("State property 'base.level' is not defined", error->GetMessage());
}
TEST_F(StateManagerTest, GetAndClearRecordedStateChanges) {
- base::Time timestamp = base::Time::Now();
- EXPECT_CALL(mock_state_change_queue_, NotifyPropertiesUpdated(timestamp, _))
+ EXPECT_CALL(mock_state_change_queue_, NotifyPropertiesUpdated(timestamp_, _))
.WillOnce(Return(true));
- ASSERT_TRUE(mgr_->SetPropertyValue(
- "terminator.target", std::string{"John Connor"}, timestamp, nullptr));
+ ASSERT_TRUE(SetPropertyValue("terminator.target", std::string{"John Connor"},
+ nullptr));
std::vector<StateChange> expected_val;
expected_val.emplace_back(
- timestamp,
+ timestamp_,
native_types::Object{{"terminator.target",
unittests::make_string_prop_value("John Connor")}});
EXPECT_CALL(mock_state_change_queue_, GetAndClearRecordedStateChanges())
@@ -194,4 +204,28 @@
changes.back().changed_properties);
}
+TEST_F(StateManagerTest, SetProperties) {
+ native_types::Object expected_prop_set{
+ {"base.manufacturer", unittests::make_string_prop_value("No Name")},
+ };
+ EXPECT_CALL(mock_state_change_queue_,
+ NotifyPropertiesUpdated(_, expected_prop_set))
+ .WillOnce(Return(true));
+
+ EXPECT_CALL(*this, OnStateChanged()).Times(1);
+ ASSERT_TRUE(mgr_->SetProperties(
+ {{"base.manufacturer", std::string("No Name")}}, nullptr));
+
+ auto expected = R"({
+ 'base': {
+ 'manufacturer': 'No Name',
+ 'serialNumber': 'T1000'
+ },
+ 'terminator': {
+ 'target': ''
+ }
+ })";
+ EXPECT_JSON_EQ(expected, *mgr_->GetStateValuesAsJson(nullptr));
+}
+
} // namespace buffet
