buffet: Add the ability to change command visibility Now it is possible to change the visibility of command and make it visible to cloud only, local only, both or none. BUG=brillo:797 TEST=`FEATURES=test emerge-link buffet` Change-Id: I81d526b3d43adf5d6cd03a4e31a31e1494ff5c1b Reviewed-on: https://chromium-review.googlesource.com/266396 Trybot-Ready: Alex Vakulenko <avakulenko@chromium.org> Tested-by: Alex Vakulenko <avakulenko@chromium.org> Reviewed-by: Vitaly Buka <vitalybuka@chromium.org> Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
diff --git a/buffet/buffet_client.cc b/buffet/buffet_client.cc index 1c930a4..675b4dc 100644 --- a/buffet/buffet_client.cc +++ b/buffet/buffet_client.cc
@@ -20,6 +20,7 @@ #include <chromeos/dbus/data_serialization.h> #include <chromeos/dbus/dbus_method_invoker.h> #include <chromeos/errors/error.h> +#include <chromeos/strings/string_utils.h> #include <chromeos/variant_dictionary.h> #include <dbus/bus.h> #include <dbus/message.h> @@ -45,6 +46,7 @@ - UpdateState prop_name prop_value - GetState - PendingCommands + - SetCommandVisibility pkg1.cmd1[,pkg2.cm2,...] [all|cloud|local|none] )"); } @@ -227,6 +229,12 @@ base::Bind(&Daemon::CallGetPendingCommands, weak_factory_.GetWeakPtr()), base::TimeDelta::FromMilliseconds(100)); + } else if (command.compare("SetCommandVisibility") == 0 || + command.compare("cv") == 0) { + if (!CheckArgs(command, args, 2)) + return EX_USAGE; + job = base::Bind(&Daemon::CallSetCommandVisibility, + weak_factory_.GetWeakPtr(), args.front(), args.back()); } else { fprintf(stderr, "Unknown command: '%s'\n", command.c_str()); return EX_USAGE; @@ -374,6 +382,18 @@ OnJobComplete(); } + void CallSetCommandVisibility(const std::string& command_list, + const std::string& visibility, + ManagerProxy* manager_proxy) { + ErrorPtr error; + std::vector<std::string> commands = + chromeos::string_utils::Split(command_list, ",", true, true); + if (!manager_proxy->SetCommandVisibility(commands, visibility, &error)) { + return ReportError(error.get()); + } + OnJobComplete(); + } + std::unique_ptr<org::chromium::Buffet::ObjectManagerProxy> object_manager_; int exit_code_{EX_OK}; base::CancelableCallback<void()> timeout_task_;
diff --git a/buffet/commands/command_dictionary.cc b/buffet/commands/command_dictionary.cc index f2b6f2f..3ab57a5 100644 --- a/buffet/commands/command_dictionary.cc +++ b/buffet/commands/command_dictionary.cc
@@ -235,6 +235,12 @@ return (pair != definitions_.end()) ? pair->second.get() : nullptr; } +CommandDefinition* CommandDictionary::FindCommand( + const std::string& command_name) { + auto pair = definitions_.find(command_name); + return (pair != definitions_.end()) ? pair->second.get() : nullptr; +} + void CommandDictionary::Clear() { definitions_.clear(); }
diff --git a/buffet/commands/command_dictionary.h b/buffet/commands/command_dictionary.h index 52e1a10..7668fba 100644 --- a/buffet/commands/command_dictionary.h +++ b/buffet/commands/command_dictionary.h
@@ -78,6 +78,7 @@ void Clear(); // Finds a definition for the given command. const CommandDefinition* FindCommand(const std::string& command_name) const; + CommandDefinition* FindCommand(const std::string& command_name); private: using CommandMap = std::map<std::string, std::unique_ptr<CommandDefinition>>;
diff --git a/buffet/commands/command_manager.cc b/buffet/commands/command_manager.cc index 6facaf9..dd90e7d 100644 --- a/buffet/commands/command_manager.cc +++ b/buffet/commands/command_manager.cc
@@ -104,4 +104,35 @@ return command_queue_.Find(id); } +bool CommandManager::SetCommandVisibility( + const std::vector<std::string>& command_names, + CommandDefinition::Visibility visibility, + chromeos::ErrorPtr* error) { + if (command_names.empty()) + return true; + + std::vector<CommandDefinition*> definitions; + definitions.reserve(command_names.size()); + + // Find/validate command definitions first. + for (const std::string& name : command_names) { + CommandDefinition* def = dictionary_.FindCommand(name); + if (!def) { + chromeos::Error::AddToPrintf(error, FROM_HERE, errors::commands::kDomain, + errors::commands::kInvalidCommandName, + "Command '%s' is unknown", name.c_str()); + return false; + } + definitions.push_back(def); + } + + // Now that we know that all the command names were valid, + // update the respective commands' visibility. + for (CommandDefinition* def : definitions) { + def->SetVisibility(visibility); + } + on_command_changed_.Notify(); + return true; +} + } // namespace buffet
diff --git a/buffet/commands/command_manager.h b/buffet/commands/command_manager.h index 3e23cbf..072d055 100644 --- a/buffet/commands/command_manager.h +++ b/buffet/commands/command_manager.h
@@ -7,6 +7,7 @@ #include <memory> #include <string> +#include <vector> #include <base/callback.h> #include <base/callback_list.h> @@ -49,7 +50,8 @@ explicit CommandManager(CommandDispachInterface* dispatch_interface); // Sets callback which is called when command definitions is changed. - CallbackToken AddOnCommandDefChanged(const base::Closure& callback) { + CallbackToken AddOnCommandDefChanged( + const base::Closure& callback) WARN_UNUSED_RESULT { return CallbackToken{on_command_changed_.Add(callback).release()}; } @@ -102,6 +104,11 @@ // for a long period of time. CommandInstance* FindCommand(const std::string& id) const; + // Changes the visibility of commands. + bool SetCommandVisibility(const std::vector<std::string>& command_names, + CommandDefinition::Visibility visibility, + chromeos::ErrorPtr* error); + private: CommandDictionary base_dictionary_; // Base/std command definitions/schemas. CommandDictionary dictionary_; // Command definitions/schemas.
diff --git a/buffet/commands/command_manager_unittest.cc b/buffet/commands/command_manager_unittest.cc index 2d4fed8..57e0d06 100644 --- a/buffet/commands/command_manager_unittest.cc +++ b/buffet/commands/command_manager_unittest.cc
@@ -7,6 +7,7 @@ #include <base/files/file_util.h> #include <base/files/scoped_temp_dir.h> #include <base/json/json_writer.h> +#include <chromeos/bind_lambda.h> #include <gtest/gtest.h> #include "buffet/commands/unittest_utils.h" @@ -156,3 +157,61 @@ EXPECT_NE(nullptr, manager.GetCommandDictionary().FindCommand("test._yo")); } + +TEST(CommandManager, UpdateCommandVisibility) { + buffet::CommandManager manager; + int update_count = 0; + auto on_command_change = [&update_count]() { update_count++; }; + auto token = manager.AddOnCommandDefChanged(base::Bind(on_command_change)); + + auto json = CreateDictionaryValue(R"({ + 'foo': { + '_baz': { + 'parameters': {}, + 'results': {} + }, + '_bar': { + 'parameters': {}, + 'results': {} + } + }, + 'bar': { + '_quux': { + 'parameters': {}, + 'results': {}, + 'visibility': 'none' + } + } + })"); + ASSERT_TRUE(manager.LoadCommands(*json, "test", nullptr)); + EXPECT_EQ(1, update_count); + const buffet::CommandDictionary& dict = manager.GetCommandDictionary(); + EXPECT_TRUE(manager.SetCommandVisibility( + {"foo._baz"}, + buffet::CommandDefinition::Visibility::GetLocal(), nullptr)); + EXPECT_EQ(2, update_count); + EXPECT_EQ("local", dict.FindCommand("foo._baz")->GetVisibility().ToString()); + EXPECT_EQ("all", dict.FindCommand("foo._bar")->GetVisibility().ToString()); + EXPECT_EQ("none", dict.FindCommand("bar._quux")->GetVisibility().ToString()); + + chromeos::ErrorPtr error; + ASSERT_FALSE(manager.SetCommandVisibility( + {"foo._baz", "foo._bar", "test.cmd"}, + buffet::CommandDefinition::Visibility::GetLocal(), &error)); + EXPECT_EQ(buffet::errors::commands::kInvalidCommandName, error->GetCode()); + EXPECT_EQ("Command 'test.cmd' is unknown", error->GetMessage()); + // The visibility state of commands shouldn't have changed. + EXPECT_EQ(2, update_count); + EXPECT_EQ("local", dict.FindCommand("foo._baz")->GetVisibility().ToString()); + EXPECT_EQ("all", dict.FindCommand("foo._bar")->GetVisibility().ToString()); + EXPECT_EQ("none", dict.FindCommand("bar._quux")->GetVisibility().ToString()); + + EXPECT_TRUE(manager.SetCommandVisibility( + {"foo._baz", "bar._quux"}, + buffet::CommandDefinition::Visibility::GetCloud(), nullptr)); + EXPECT_EQ(3, update_count); + EXPECT_EQ("cloud", dict.FindCommand("foo._baz")->GetVisibility().ToString()); + EXPECT_EQ("all", dict.FindCommand("foo._bar")->GetVisibility().ToString()); + EXPECT_EQ("cloud", dict.FindCommand("bar._quux")->GetVisibility().ToString()); +} +
diff --git a/buffet/dbus_bindings/org.chromium.Buffet.Manager.xml b/buffet/dbus_bindings/org.chromium.Buffet.Manager.xml index 0d9c485..6cf9535 100644 --- a/buffet/dbus_bindings/org.chromium.Buffet.Manager.xml +++ b/buffet/dbus_bindings/org.chromium.Buffet.Manager.xml
@@ -45,6 +45,23 @@ <arg name="json_command" type="s" direction="out"/> <annotation name="org.chromium.DBus.Method.Kind" value="async"/> </method> + <method name="SetCommandVisibility"> + <tp:docstring> + Changes the visibility of command definition. + Parameters: + names - a list of the full names of commands to update + (each command name should look like 'base.identify'). + visibility - the new visibility state of the command: + "none" - Command is hidden/unavailable. + "local" - Command is available to local clients only. + "cloud" - Command is available to cloud clients only. + "all" - Command is available to both local and cloud + clients. + </tp:docstring> + <arg name="names" type="as" direction="in"/> + <arg name="visibility" type="s" direction="in"/> + <annotation name="org.chromium.DBus.Method.Kind" value="async"/> + </method> <method name="TestMethod"> <arg name="message" type="s" direction="in"/> <arg name="echoed_message" type="s" direction="out"/>
diff --git a/buffet/manager.cc b/buffet/manager.cc index 860e880..b83ccb9 100644 --- a/buffet/manager.cc +++ b/buffet/manager.cc
@@ -22,6 +22,7 @@ #include <dbus/values_util.h> #include "buffet/commands/command_instance.h" +#include "buffet/commands/schema_constants.h" #include "buffet/states/state_change_queue.h" #include "buffet/states/state_manager.h" #include "buffet/storage_impls.h" @@ -207,6 +208,23 @@ response->Return(command_str); } +void Manager::SetCommandVisibility( + scoped_ptr<chromeos::dbus_utils::DBusMethodResponse<>> response, + const std::vector<std::string>& in_names, + const std::string& in_visibility) { + CommandDefinition::Visibility visibility; + chromeos::ErrorPtr error; + if (!visibility.FromString(in_visibility, &error)) { + response->ReplyWithError(error.get()); + return; + } + if (!command_manager_->SetCommandVisibility(in_names, visibility, &error)) { + response->ReplyWithError(error.get()); + return; + } + response->Return(); +} + std::string Manager::TestMethod(const std::string& message) { LOG(INFO) << "Received call to test method: " << message; return message;
diff --git a/buffet/manager.h b/buffet/manager.h index a00ba20..3cf7021 100644 --- a/buffet/manager.h +++ b/buffet/manager.h
@@ -8,6 +8,7 @@ #include <map> #include <memory> #include <string> +#include <vector> #include <base/files/file_path.h> #include <base/macros.h> @@ -72,6 +73,10 @@ const std::string& json_command) override; void GetCommand(DBusMethodResponse<std::string> response, const std::string& id) override; + void SetCommandVisibility( + scoped_ptr<chromeos::dbus_utils::DBusMethodResponse<>> response, + const std::vector<std::string>& in_names, + const std::string& in_visibility) override; std::string TestMethod(const std::string& message) override; void OnCommandDefsChanged();