buffet: Add command_def filtering by command visibility
When updating cloud or local client with the device's CDD make sure
to send only the command definitions that are available to particular
client (local vs cloud).
Also made it possible to subscribe to command definition change notifications
for more than one listener (using base::CallbackList) so that both the local
and cloud adapters can notify the respective clients about command visibility
changes (the actual API to change the command visibility is coming in a
follow-up CL).
BUG=brillo:797
TEST=`FEATURES=test emerge-link buffet`
Change-Id: I6bec36633ababcb534012abad2c37a3502d8faf4
Reviewed-on: https://chromium-review.googlesource.com/266209
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Trybot-Ready: Alex Vakulenko <avakulenko@chromium.org>
Tested-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
diff --git a/buffet/commands/command_dictionary.cc b/buffet/commands/command_dictionary.cc
index 2469e63..f2b6f2f 100644
--- a/buffet/commands/command_dictionary.cc
+++ b/buffet/commands/command_dictionary.cc
@@ -195,9 +195,16 @@
}
std::unique_ptr<base::DictionaryValue> CommandDictionary::GetCommandsAsJson(
- bool full_schema, chromeos::ErrorPtr* error) const {
+ const std::function<bool(const CommandDefinition*)>& filter,
+ bool full_schema,
+ chromeos::ErrorPtr* error) const {
std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue);
for (const auto& pair : definitions_) {
+ // Check if the command definition has the desired visibility.
+ // If not, then skip it.
+ if (!filter(pair.second.get()))
+ continue;
+
std::unique_ptr<base::DictionaryValue> definition =
pair.second->GetParameters()->ToJson(full_schema, error);
if (!definition) {
diff --git a/buffet/commands/command_dictionary.h b/buffet/commands/command_dictionary.h
index db68c8a..52e1a10 100644
--- a/buffet/commands/command_dictionary.h
+++ b/buffet/commands/command_dictionary.h
@@ -7,6 +7,7 @@
#include <map>
#include <memory>
+#include <set>
#include <string>
#include <vector>
@@ -57,12 +58,18 @@
const CommandDictionary* base_commands,
chromeos::ErrorPtr* error);
// Converts all the command definitions to a JSON object for CDD/Device
- // draft. |full_schema| specifies whether full command definitions must
- // be generated (true) for CDD or only overrides from the base schema (false).
+ // draft.
+ // |filter| is a predicate used to filter out the command definitions to
+ // be returned by this method. Only command definitions for which the
+ // predicate returns true will be included in the resulting JSON.
+ // |full_schema| specifies whether full command definitions must be generated
+ // (true) for CDD or only overrides from the base schema (false).
// Returns empty unique_ptr in case of an error and fills in the additional
// error details in |error|.
std::unique_ptr<base::DictionaryValue> GetCommandsAsJson(
- bool full_schema, chromeos::ErrorPtr* error) const;
+ const std::function<bool(const CommandDefinition*)>& filter,
+ bool full_schema,
+ chromeos::ErrorPtr* error) const;
// Returns the number of command definitions in the dictionary.
size_t GetSize() const { return definitions_.size(); }
// Checks if the dictionary has no command definitions.
diff --git a/buffet/commands/command_dictionary_unittest.cc b/buffet/commands/command_dictionary_unittest.cc
index 143b53e..c50a2ae 100644
--- a/buffet/commands/command_dictionary_unittest.cc
+++ b/buffet/commands/command_dictionary_unittest.cc
@@ -238,13 +238,16 @@
buffet::CommandDictionary dict;
dict.LoadCommands(*json, "device", &base_dict, nullptr);
- json = dict.GetCommandsAsJson(false, nullptr);
+ using buffet::CommandDefinition;
+ json = dict.GetCommandsAsJson(
+ [](const CommandDefinition* def) { return true; }, false, nullptr);
EXPECT_NE(nullptr, json.get());
EXPECT_EQ("{'base':{'reboot':{'parameters':{'delay':{'minimum':10}}}},"
"'robot':{'_jump':{'parameters':{'_height':'integer'}}}}",
buffet::unittests::ValueToString(json.get()));
- json = dict.GetCommandsAsJson(true, nullptr);
+ json = dict.GetCommandsAsJson(
+ [](const CommandDefinition* def) { return true; }, true, nullptr);
EXPECT_NE(nullptr, json.get());
EXPECT_EQ("{'base':{'reboot':{'parameters':{'delay':{"
"'maximum':100,'minimum':10,'type':'integer'}}}},"
@@ -252,6 +255,106 @@
buffet::unittests::ValueToString(json.get()));
}
+TEST(CommandDictionary, GetCommandsAsJsonWithVisibility) {
+ auto json = buffet::unittests::CreateDictionaryValue(R"({
+ 'test': {
+ 'command1': {
+ 'parameters': {},
+ 'results': {},
+ 'visibility': 'none'
+ },
+ 'command2': {
+ 'parameters': {},
+ 'results': {},
+ 'visibility': 'local'
+ },
+ 'command3': {
+ 'parameters': {},
+ 'results': {},
+ 'visibility': 'cloud'
+ },
+ 'command4': {
+ 'parameters': {},
+ 'results': {},
+ 'visibility': 'all'
+ },
+ 'command5': {
+ 'parameters': {},
+ 'results': {},
+ 'visibility': 'none'
+ },
+ 'command6': {
+ 'parameters': {},
+ 'results': {},
+ 'visibility': 'local'
+ },
+ 'command7': {
+ 'parameters': {},
+ 'results': {},
+ 'visibility': 'cloud'
+ },
+ 'command8': {
+ 'parameters': {},
+ 'results': {},
+ 'visibility': 'all'
+ }
+ }
+ })");
+ buffet::CommandDictionary dict;
+ ASSERT_TRUE(dict.LoadCommands(*json, "test", nullptr, nullptr));
+
+ using buffet::CommandDefinition;
+ json = dict.GetCommandsAsJson(
+ [](const CommandDefinition* def) { return true; }, false, nullptr);
+ ASSERT_NE(nullptr, json.get());
+ EXPECT_EQ("{'test':{"
+ "'command1':{'parameters':{}},"
+ "'command2':{'parameters':{}},"
+ "'command3':{'parameters':{}},"
+ "'command4':{'parameters':{}},"
+ "'command5':{'parameters':{}},"
+ "'command6':{'parameters':{}},"
+ "'command7':{'parameters':{}},"
+ "'command8':{'parameters':{}}"
+ "}}",
+ buffet::unittests::ValueToString(json.get()));
+
+ json = dict.GetCommandsAsJson(
+ [](const CommandDefinition* def) { return def->GetVisibility().local; },
+ false, nullptr);
+ ASSERT_NE(nullptr, json.get());
+ EXPECT_EQ("{'test':{"
+ "'command2':{'parameters':{}},"
+ "'command4':{'parameters':{}},"
+ "'command6':{'parameters':{}},"
+ "'command8':{'parameters':{}}"
+ "}}",
+ buffet::unittests::ValueToString(json.get()));
+
+ json = dict.GetCommandsAsJson(
+ [](const CommandDefinition* def) { return def->GetVisibility().cloud; },
+ false, nullptr);
+ ASSERT_NE(nullptr, json.get());
+ EXPECT_EQ("{'test':{"
+ "'command3':{'parameters':{}},"
+ "'command4':{'parameters':{}},"
+ "'command7':{'parameters':{}},"
+ "'command8':{'parameters':{}}"
+ "}}",
+ buffet::unittests::ValueToString(json.get()));
+
+ json = dict.GetCommandsAsJson(
+ [](const CommandDefinition* def) {
+ return def->GetVisibility().local && def->GetVisibility().cloud;
+ }, false, nullptr);
+ ASSERT_NE(nullptr, json.get());
+ EXPECT_EQ("{'test':{"
+ "'command4':{'parameters':{}},"
+ "'command8':{'parameters':{}}"
+ "}}",
+ buffet::unittests::ValueToString(json.get()));
+}
+
TEST(CommandDictionary, LoadCommandsWithVisibility) {
buffet::CommandDictionary dict;
auto json = CreateDictionaryValue(R"({
@@ -259,7 +362,7 @@
'command1': {
'parameters': {},
'results': {},
- 'visibility':''
+ 'visibility':'none'
},
'command2': {
'parameters': {},
@@ -312,7 +415,7 @@
'command1': {
'parameters': {},
'results': {},
- 'visibility':''
+ 'visibility':'none'
},
'command2': {
'parameters': {},
diff --git a/buffet/commands/command_manager.cc b/buffet/commands/command_manager.cc
index f7533ca..6facaf9 100644
--- a/buffet/commands/command_manager.cc
+++ b/buffet/commands/command_manager.cc
@@ -53,8 +53,7 @@
chromeos::ErrorPtr* error) {
bool result =
dictionary_.LoadCommands(json, category, &base_dictionary_, error);
- if (!on_command_defs_changed_.is_null())
- on_command_defs_changed_.Run();
+ on_command_changed_.Notify();
return result;
}
diff --git a/buffet/commands/command_manager.h b/buffet/commands/command_manager.h
index a703ab3..3e23cbf 100644
--- a/buffet/commands/command_manager.h
+++ b/buffet/commands/command_manager.h
@@ -8,6 +8,8 @@
#include <memory>
#include <string>
+#include <base/callback.h>
+#include <base/callback_list.h>
#include <base/files/file_path.h>
#include <base/macros.h>
#include <base/memory/weak_ptr.h>
@@ -32,6 +34,13 @@
// dispatched to the device.
class CommandManager final {
public:
+ // A token given by CommandManager in response to AddOnCommandDefChanged().
+ // When the CallbackToken is destroyed, the registered notification
+ // callback associated with it will automatically be removed from the command
+ // manager's callback list.
+ using CallbackToken =
+ std::unique_ptr<base::CallbackList<void()>::Subscription>;
+
CommandManager();
explicit CommandManager(
const base::WeakPtr<chromeos::dbus_utils::ExportedObjectManager>&
@@ -40,8 +49,8 @@
explicit CommandManager(CommandDispachInterface* dispatch_interface);
// Sets callback which is called when command definitions is changed.
- void SetOnCommandDefChanged(const base::Closure& on_command_defs_changed) {
- on_command_defs_changed_ = on_command_defs_changed;
+ CallbackToken AddOnCommandDefChanged(const base::Closure& callback) {
+ return CallbackToken{on_command_changed_.Add(callback).release()};
}
// Returns the command definitions for the device.
@@ -98,7 +107,7 @@
CommandDictionary dictionary_; // Command definitions/schemas.
CommandQueue command_queue_;
DBusCommandDispacher command_dispatcher_;
- base::Closure on_command_defs_changed_;
+ base::CallbackList<void()> on_command_changed_;
DISALLOW_COPY_AND_ASSIGN(CommandManager);
};
diff --git a/buffet/device_registration_info.cc b/buffet/device_registration_info.cc
index a9601cd..d91e27c 100644
--- a/buffet/device_registration_info.cc
+++ b/buffet/device_registration_info.cc
@@ -5,6 +5,7 @@
#include "buffet/device_registration_info.h"
#include <memory>
+#include <set>
#include <utility>
#include <vector>
@@ -137,6 +138,9 @@
xmpp_enabled_{xmpp_enabled},
manager_{manager} {
OnConfigChanged();
+ command_changed_callback_token_ = command_manager_->AddOnCommandDefChanged(
+ base::Bind(&DeviceRegistrationInfo::OnCommandDefsChanged,
+ weak_factory_.GetWeakPtr()));
}
DeviceRegistrationInfo::~DeviceRegistrationInfo() = default;
@@ -386,8 +390,10 @@
std::unique_ptr<base::DictionaryValue>
DeviceRegistrationInfo::BuildDeviceResource(chromeos::ErrorPtr* error) {
- std::unique_ptr<base::DictionaryValue> commands =
- command_manager_->GetCommandDictionary().GetCommandsAsJson(true, error);
+ // Limit only to commands that are visible to the cloud.
+ auto commands = command_manager_->GetCommandDictionary().GetCommandsAsJson(
+ [](const CommandDefinition* def) { return def->GetVisibility().cloud; },
+ true, error);
if (!commands)
return nullptr;
@@ -782,6 +788,7 @@
void DeviceRegistrationInfo::UpdateDeviceResource(
const base::Closure& on_success,
const CloudRequestErrorCallback& on_failure) {
+ VLOG(1) << "Updating GCD server with CDD...";
std::unique_ptr<base::DictionaryValue> device_resource =
BuildDeviceResource(nullptr);
if (!device_resource)
@@ -977,4 +984,13 @@
manager_->SetLocation(config_->location());
}
+void DeviceRegistrationInfo::OnCommandDefsChanged() {
+ VLOG(1) << "CommandDefinitionChanged notification received";
+ if (!HaveRegistrationCredentials(nullptr))
+ return;
+
+ UpdateDeviceResource(base::Bind(&base::DoNothing),
+ base::Bind(&IgnoreCloudError));
+}
+
} // namespace buffet
diff --git a/buffet/device_registration_info.h b/buffet/device_registration_info.h
index e3a4b80..f153cda 100644
--- a/buffet/device_registration_info.h
+++ b/buffet/device_registration_info.h
@@ -21,6 +21,7 @@
#include <chromeos/http/http_transport.h>
#include "buffet/buffet_config.h"
+#include "buffet/commands/command_manager.h"
#include "buffet/registration_status.h"
#include "buffet/storage_interface.h"
#include "buffet/xmpp/xmpp_client.h"
@@ -43,7 +44,6 @@
namespace buffet {
-class CommandManager;
class StateManager;
extern const char kErrorDomainOAuth2[];
@@ -215,7 +215,7 @@
void PublishStateUpdates();
- // Builds Cloud API devices collection REST resouce which matches
+ // Builds Cloud API devices collection REST resource which matches
// current state of the device including command definitions
// for all supported commands and current device state.
std::unique_ptr<base::DictionaryValue> BuildDeviceResource(
@@ -225,6 +225,9 @@
void SetDeviceId(const std::string& device_id);
void OnConfigChanged();
+ // Callback called when command definitions are changed to re-publish new CDD.
+ void OnCommandDefsChanged();
+
// Data that is cached here, persisted in the state store.
std::string refresh_token_;
std::string device_id_;
@@ -243,6 +246,10 @@
// Device state manager.
std::shared_ptr<StateManager> state_manager_;
+ // Token given by Command Manager to track the registered Command Definition
+ // change callback.
+ CommandManager::CallbackToken command_changed_callback_token_;
+
std::unique_ptr<BuffetConfig> config_;
const bool xmpp_enabled_;
diff --git a/buffet/manager.cc b/buffet/manager.cc
index 93d3029..860e880 100644
--- a/buffet/manager.cc
+++ b/buffet/manager.cc
@@ -5,6 +5,7 @@
#include "buffet/manager.h"
#include <map>
+#include <set>
#include <string>
#include <base/bind.h>
@@ -21,7 +22,6 @@
#include <dbus/values_util.h>
#include "buffet/commands/command_instance.h"
-#include "buffet/commands/command_manager.h"
#include "buffet/states/state_change_queue.h"
#include "buffet/states/state_manager.h"
#include "buffet/storage_impls.h"
@@ -52,7 +52,7 @@
const AsyncEventSequencer::CompletionAction& cb) {
command_manager_ =
std::make_shared<CommandManager>(dbus_object_.GetObjectManager());
- command_manager_->SetOnCommandDefChanged(
+ command_changed_callback_token_ = command_manager_->AddOnCommandDefChanged(
base::Bind(&Manager::OnCommandDefsChanged, base::Unretained(this)));
command_manager_->Startup(base::FilePath{"/etc/buffet"},
test_definitions_path);
@@ -222,8 +222,11 @@
void Manager::OnCommandDefsChanged() {
chromeos::ErrorPtr error;
- std::unique_ptr<base::DictionaryValue> commands =
- command_manager_->GetCommandDictionary().GetCommandsAsJson(true, &error);
+ // Limit only to commands that are visible to the local clients.
+ auto commands = command_manager_->GetCommandDictionary().GetCommandsAsJson(
+ [](const buffet::CommandDefinition* def) {
+ return def->GetVisibility().local;
+ }, true, &error);
CHECK(commands);
std::string json;
base::JSONWriter::WriteWithOptions(commands.get(),
diff --git a/buffet/manager.h b/buffet/manager.h
index 177d3d1..a00ba20 100644
--- a/buffet/manager.h
+++ b/buffet/manager.h
@@ -18,6 +18,7 @@
#include <chromeos/dbus/exported_property_set.h>
#include <chromeos/errors/error.h>
+#include "buffet/commands/command_manager.h"
#include "buffet/device_registration_info.h"
#include "buffet/org.chromium.Buffet.Manager.h"
@@ -29,7 +30,6 @@
namespace buffet {
-class CommandManager;
class StateChangeQueue;
class StateManager;
class BuffetConfig;
@@ -84,6 +84,10 @@
std::shared_ptr<StateManager> state_manager_;
std::unique_ptr<DeviceRegistrationInfo> device_info_;
+ // Token given by Command Manager to track the registered Command Definition
+ // change callback.
+ CommandManager::CallbackToken command_changed_callback_token_;
+
DISALLOW_COPY_AND_ASSIGN(Manager);
};