Add Device::SetCommandHandler

BUG:24267885
Change-Id: Ibcc1af050ad0de4130dfc3349545f6ab1ddc3f49
Reviewed-on: https://weave-review.googlesource.com/1250
Reviewed-by: Vitaly Buka <vitalybuka@google.com>
diff --git a/libweave/examples/ubuntu/main.cc b/libweave/examples/ubuntu/main.cc
index 5c84d9c..c28d164 100644
--- a/libweave/examples/ubuntu/main.cc
+++ b/libweave/examples/ubuntu/main.cc
@@ -34,66 +34,85 @@
 class CommandHandler {
  public:
   explicit CommandHandler(weave::Device* device) : device_{device} {
-    device->AddCommandAddedCallback(base::Bind(&CommandHandler::OnNewCommand,
-                                               weak_ptr_factory_.GetWeakPtr()));
+    device->AddCommandHandler("_greeter._greet",
+                              base::Bind(&CommandHandler::OnGreetCommand,
+                                         weak_ptr_factory_.GetWeakPtr()));
+    device->AddCommandHandler("_ledflasher._set",
+                              base::Bind(&CommandHandler::OnFlasherSetCommand,
+                                         weak_ptr_factory_.GetWeakPtr()));
+    device->AddCommandHandler(
+        "_ledflasher._toggle",
+        base::Bind(&CommandHandler::OnFlasherToggleCommand,
+                   weak_ptr_factory_.GetWeakPtr()));
+    device->AddCommandHandler("",
+                              base::Bind(&CommandHandler::OnUnhandledCommand,
+                                         weak_ptr_factory_.GetWeakPtr()));
   }
 
  private:
-  void OnNewCommand(weave::Command* cmd) {
+  void OnGreetCommand(weave::Command* cmd) {
     LOG(INFO) << "received command: " << cmd->GetName();
-    if (cmd->GetName() == "_greeter._greet") {
-      std::string name;
-      if (!cmd->GetParameters()->GetString("_name", &name))
-        name = "anonymous";
+    std::string name;
+    if (!cmd->GetParameters()->GetString("_name", &name))
+      name = "anonymous";
 
-      LOG(INFO) << cmd->GetName() << " command in progress";
-      cmd->SetProgress(base::DictionaryValue{}, nullptr);
+    LOG(INFO) << cmd->GetName() << " command in progress";
+    cmd->SetProgress(base::DictionaryValue{}, nullptr);
 
-      base::DictionaryValue result;
-      result.SetString("_greeting", "Hello " + name);
-      cmd->SetResults(result, nullptr);
-      LOG(INFO) << cmd->GetName() << " command finished: " << result;
+    base::DictionaryValue result;
+    result.SetString("_greeting", "Hello " + name);
+    cmd->SetResults(result, nullptr);
+    LOG(INFO) << cmd->GetName() << " command finished: " << result;
 
-      base::DictionaryValue state;
-      state.SetIntegerWithoutPathExpansion("_greeter._greetings_counter",
-                                           ++counter_);
-      device_->SetStateProperties(state, nullptr);
+    base::DictionaryValue state;
+    state.SetIntegerWithoutPathExpansion("_greeter._greetings_counter",
+                                         ++counter_);
+    device_->SetStateProperties(state, nullptr);
 
-      LOG(INFO) << "New state: " << *device_->GetState();
+    LOG(INFO) << "New state: " << *device_->GetState();
 
-      cmd->Done();
-    } else if (cmd->GetName() == "_ledflasher._set") {
-      int32_t led_index;
-      bool cmd_value;
-      if (cmd->GetParameters()->GetInteger("_led", &led_index) &&
-          cmd->GetParameters()->GetBoolean("_on", &cmd_value)) {
-        // Display this command in terminal
-        LOG(INFO) << cmd->GetName() << " _led: " << led_index
-                  << ", _on: " << (cmd_value ? "true" : "false");
+    cmd->Done();
+  }
 
-        led_index--;
-        int new_state = cmd_value ? 1 : 0;
-        int cur_state = led_status_[led_index];
-        led_status_[led_index] = new_state;
+  void OnFlasherSetCommand(weave::Command* cmd) {
+    LOG(INFO) << "received command: " << cmd->GetName();
+    int32_t led_index;
+    bool cmd_value;
+    if (cmd->GetParameters()->GetInteger("_led", &led_index) &&
+        cmd->GetParameters()->GetBoolean("_on", &cmd_value)) {
+      // Display this command in terminal
+      LOG(INFO) << cmd->GetName() << " _led: " << led_index
+                << ", _on: " << (cmd_value ? "true" : "false");
 
-        if (cmd_value != cur_state) {
-          UpdateLedState();
-        }
-      }
-      cmd->Done();
-    } else if (cmd->GetName() == "_ledflasher._toggle") {
-      int32_t led_index;
-      if (cmd->GetParameters()->GetInteger("_led", &led_index)) {
-        LOG(INFO) << cmd->GetName() << " _led: " << led_index;
-        led_index--;
-        led_status_[led_index] = ~led_status_[led_index];
+      led_index--;
+      int new_state = cmd_value ? 1 : 0;
+      int cur_state = led_status_[led_index];
+      led_status_[led_index] = new_state;
 
+      if (cmd_value != cur_state) {
         UpdateLedState();
       }
-      cmd->Done();
-    } else {
-      LOG(INFO) << cmd->GetName() << " unimplemented command: ignored";
+      return cmd->Done();
     }
+    cmd->Abort();
+  }
+
+  void OnFlasherToggleCommand(weave::Command* cmd) {
+    LOG(INFO) << "received command: " << cmd->GetName();
+    int32_t led_index;
+    if (cmd->GetParameters()->GetInteger("_led", &led_index)) {
+      LOG(INFO) << cmd->GetName() << " _led: " << led_index;
+      led_index--;
+      led_status_[led_index] = ~led_status_[led_index];
+
+      UpdateLedState();
+      return cmd->Done();
+    }
+    cmd->Abort();
+  }
+
+  void OnUnhandledCommand(weave::Command* cmd) {
+    LOG(INFO) << cmd->GetName() << " unimplemented command: ignored";
   }
 
   void UpdateLedState(void) {
diff --git a/libweave/include/weave/device.h b/libweave/include/weave/device.h
index ec8d4f5..d8b6963 100644
--- a/libweave/include/weave/device.h
+++ b/libweave/include/weave/device.h
@@ -45,11 +45,16 @@
   virtual void AddSettingsChangedCallback(
       const SettingsChangedCallback& callback) = 0;
 
-  // Callback type for AddCommandAddedCallback.
-  using CommandCallback = base::Callback<void(Command*)>;
+  // Callback type for AddCommandHandler.
+  using CommandHandlerCallback = base::Callback<void(Command*)>;
 
-  // Adds notification callback for a new command being added to the queue.
-  virtual void AddCommandAddedCallback(const CommandCallback& callback) = 0;
+  // Sets handler for new commands added to the queue.
+  // |command_name| is the full command name of the command to handle. e.g.
+  // "base.reboot". Each command can have no more than one handler.
+  // Empty |command_name| sets default handler for all unhanded commands.
+  // No new command handlers can be set after default handler was set.
+  virtual void AddCommandHandler(const std::string& command_name,
+                                 const CommandHandlerCallback& callback) = 0;
 
   // Adds a new command to the command queue.
   virtual bool AddCommand(const base::DictionaryValue& command,
diff --git a/libweave/include/weave/test/mock_device.h b/libweave/include/weave/test/mock_device.h
index c289ca7..af149b4 100644
--- a/libweave/include/weave/test/mock_device.h
+++ b/libweave/include/weave/test/mock_device.h
@@ -21,7 +21,8 @@
   MOCK_CONST_METHOD0(GetSettings, const Settings&());
   MOCK_METHOD1(AddSettingsChangedCallback,
                void(const SettingsChangedCallback& callback));
-  MOCK_METHOD1(AddCommandAddedCallback, void(const CommandCallback&));
+  MOCK_METHOD2(AddCommandHandler,
+               void(const std::string&, const CommandHandlerCallback&));
   MOCK_METHOD3(AddCommand,
                bool(const base::DictionaryValue&, std::string*, ErrorPtr*));
   MOCK_METHOD1(FindCommand, Command*(const std::string&));
diff --git a/libweave/src/base_api_handler.cc b/libweave/src/base_api_handler.cc
index f5dccd7..e63d8cb 100644
--- a/libweave/src/base_api_handler.cc
+++ b/libweave/src/base_api_handler.cc
@@ -30,22 +30,19 @@
                                       settings.firmware_version);
   CHECK(device_->SetStateProperties(state, nullptr));
 
-  device_->AddCommandAddedCallback(base::Bind(&BaseApiHandler::OnCommandAdded,
-                                              weak_ptr_factory_.GetWeakPtr()));
-}
+  device_->AddCommandHandler(
+      "base.updateBaseConfiguration",
+      base::Bind(&BaseApiHandler::UpdateBaseConfiguration,
+                 weak_ptr_factory_.GetWeakPtr()));
 
-void BaseApiHandler::OnCommandAdded(Command* command) {
-  if (command->GetStatus() != CommandStatus::kQueued)
-    return;
-
-  if (command->GetName() == "base.updateBaseConfiguration")
-    return UpdateBaseConfiguration(command);
-
-  if (command->GetName() == "base.updateDeviceInfo")
-    return UpdateDeviceInfo(command);
+  device_->AddCommandHandler("base.updateDeviceInfo",
+                             base::Bind(&BaseApiHandler::UpdateDeviceInfo,
+                                        weak_ptr_factory_.GetWeakPtr()));
 }
 
 void BaseApiHandler::UpdateBaseConfiguration(Command* command) {
+  CHECK(command->GetStatus() == CommandStatus::kQueued)
+      << EnumToString(command->GetStatus());
   command->SetProgress(base::DictionaryValue{}, nullptr);
 
   const auto& settings = device_info_->GetSettings();
@@ -83,6 +80,8 @@
 }
 
 void BaseApiHandler::UpdateDeviceInfo(Command* command) {
+  CHECK(command->GetStatus() == CommandStatus::kQueued)
+      << EnumToString(command->GetStatus());
   command->SetProgress(base::DictionaryValue{}, nullptr);
 
   const auto& settings = device_info_->GetSettings();
diff --git a/libweave/src/base_api_handler.h b/libweave/src/base_api_handler.h
index b8a8bd1..7712de1 100644
--- a/libweave/src/base_api_handler.h
+++ b/libweave/src/base_api_handler.h
@@ -28,7 +28,6 @@
   BaseApiHandler(DeviceRegistrationInfo* device_info, Device* device);
 
  private:
-  void OnCommandAdded(Command* command);
   void UpdateBaseConfiguration(Command* command);
   void UpdateDeviceInfo(Command* command);
   bool UpdateState(const std::string& anonymous_access_role,
diff --git a/libweave/src/base_api_handler_unittest.cc b/libweave/src/base_api_handler_unittest.cc
index 2bd7741..8d30619 100644
--- a/libweave/src/base_api_handler_unittest.cc
+++ b/libweave/src/base_api_handler_unittest.cc
@@ -19,6 +19,8 @@
 #include "src/states/state_manager.h"
 
 using testing::_;
+using testing::AnyOf;
+using testing::Eq;
 using testing::Invoke;
 using testing::Return;
 using testing::StrictMock;
@@ -37,9 +39,11 @@
     EXPECT_CALL(device_, SetStateProperties(_, _))
         .WillRepeatedly(
             Invoke(state_manager_.get(), &StateManager::SetProperties));
-    EXPECT_CALL(device_, AddCommandAddedCallback(_))
-        .WillRepeatedly(Invoke(command_manager_.get(),
-                               &CommandManager::AddCommandAddedCallback));
+    EXPECT_CALL(device_, AddCommandHandler(AnyOf("base.updateBaseConfiguration",
+                                                 "base.updateDeviceInfo"),
+                                           _))
+        .WillRepeatedly(
+            Invoke(command_manager_.get(), &CommandManager::AddCommandHandler));
 
     auto state_definition = test::CreateDictionaryValue(R"({
       'base': {
@@ -65,6 +69,40 @@
     ASSERT_TRUE(
         state_manager_->LoadStateDefinition(*state_definition, nullptr));
     ASSERT_TRUE(state_manager_->LoadStateDefaults(*state_defaults, nullptr));
+
+    auto base_commands = test::CreateDictionaryValue(R"({
+      'base': {
+        'updateBaseConfiguration': {
+          'parameters': {
+            'localDiscoveryEnabled': 'boolean',
+            'localAnonymousAccessMaxRole': [ 'none', 'viewer', 'user' ],
+            'localPairingEnabled': 'boolean'
+           },
+           'results': {}
+        },
+        'updateDeviceInfo': {
+          'parameters': {
+            'description': 'string',
+            'name': {
+              'type': 'string',
+              'minLength': 1
+            },
+            'location': 'string'
+          },
+          'results': {}
+        }
+      }
+    })");
+    EXPECT_TRUE(command_manager_->LoadBaseCommands(*base_commands, nullptr));
+
+    auto handled_commands = test::CreateDictionaryValue(R"({
+      'base': {
+        'updateBaseConfiguration': {},
+        'updateDeviceInfo': {}
+      }
+    })");
+    EXPECT_TRUE(command_manager_->LoadCommands(*handled_commands, nullptr));
+
     std::unique_ptr<Config> config{new Config{&config_store_}};
     config->Load();
     dev_reg_.reset(new DeviceRegistrationInfo(command_manager_, state_manager_,
@@ -73,12 +111,6 @@
     handler_.reset(new BaseApiHandler{dev_reg_.get(), &device_});
   }
 
-  void LoadCommands(const std::string& command_definitions) {
-    auto json = test::CreateDictionaryValue(command_definitions.c_str());
-    EXPECT_TRUE(command_manager_->LoadBaseCommands(*json, nullptr));
-    EXPECT_TRUE(command_manager_->LoadCommands(*json, nullptr));
-  }
-
   void AddCommand(const std::string& command) {
     auto command_instance = CommandInstance::FromJson(
         test::CreateDictionaryValue(command.c_str()).get(),
@@ -105,19 +137,6 @@
 };
 
 TEST_F(BaseApiHandlerTest, UpdateBaseConfiguration) {
-  LoadCommands(R"({
-    'base': {
-      'updateBaseConfiguration': {
-        'parameters': {
-          'localDiscoveryEnabled': 'boolean',
-          'localAnonymousAccessMaxRole': [ 'none', 'viewer', 'user' ],
-          'localPairingEnabled': 'boolean'
-         },
-         'results': {}
-      }
-    }
-  })");
-
   const Settings& settings = dev_reg_->GetSettings();
 
   AddCommand(R"({
@@ -182,22 +201,6 @@
 }
 
 TEST_F(BaseApiHandlerTest, UpdateDeviceInfo) {
-  LoadCommands(R"({
-    'base': {
-      'updateDeviceInfo': {
-        'parameters': {
-          'description': 'string',
-          'name': {
-            'type': 'string',
-            'minLength': 1
-          },
-          'location': 'string'
-        },
-        'results': {}
-      }
-    }
-  })");
-
   AddCommand(R"({
     'name' : 'base.updateDeviceInfo',
     'parameters': {
diff --git a/libweave/src/commands/command_manager.cc b/libweave/src/commands/command_manager.cc
index 60b3d1c..9aa3385 100644
--- a/libweave/src/commands/command_manager.cc
+++ b/libweave/src/commands/command_manager.cc
@@ -176,13 +176,21 @@
 }
 
 void CommandManager::AddCommandAddedCallback(
-    const Device::CommandCallback& callback) {
+    const CommandQueue::CommandCallback& callback) {
   command_queue_.AddCommandAddedCallback(callback);
 }
 
 void CommandManager::AddCommandRemovedCallback(
-    const Device::CommandCallback& callback) {
+    const CommandQueue::CommandCallback& callback) {
   command_queue_.AddCommandRemovedCallback(callback);
 }
 
+void CommandManager::AddCommandHandler(
+    const std::string& command_name,
+    const CommandQueue::CommandCallback& callback) {
+  CHECK(command_name.empty() || base_dictionary_.FindCommand(command_name))
+      << "Command undefined: " << command_name;
+  command_queue_.AddCommandHandler(command_name, callback);
+}
+
 }  // namespace weave
diff --git a/libweave/src/commands/command_manager.h b/libweave/src/commands/command_manager.h
index 605f770..97fc73f 100644
--- a/libweave/src/commands/command_manager.h
+++ b/libweave/src/commands/command_manager.h
@@ -37,8 +37,10 @@
                   std::string* id,
                   ErrorPtr* error);
   CommandInstance* FindCommand(const std::string& id);
-  void AddCommandAddedCallback(const Device::CommandCallback& callback);
-  void AddCommandRemovedCallback(const Device::CommandCallback& callback);
+  void AddCommandAddedCallback(const CommandQueue::CommandCallback& callback);
+  void AddCommandRemovedCallback(const CommandQueue::CommandCallback& callback);
+  void AddCommandHandler(const std::string& command_name,
+                         const CommandQueue::CommandCallback& callback);
 
   // Sets callback which is called when command definitions is changed.
   void AddCommandDefChanged(const base::Closure& callback);
diff --git a/libweave/src/commands/command_queue.cc b/libweave/src/commands/command_queue.cc
index e580048..df86d77 100644
--- a/libweave/src/commands/command_queue.cc
+++ b/libweave/src/commands/command_queue.cc
@@ -13,19 +13,47 @@
 const int kRemoveCommandDelayMin = 5;
 }
 
-void CommandQueue::AddCommandAddedCallback(
-    const Device::CommandCallback& callback) {
+void CommandQueue::AddCommandAddedCallback(const CommandCallback& callback) {
   on_command_added_.push_back(callback);
   // Send all pre-existed commands.
   for (const auto& command : map_)
     callback.Run(command.second.get());
 }
 
-void CommandQueue::AddCommandRemovedCallback(
-    const Device::CommandCallback& callback) {
+void CommandQueue::AddCommandRemovedCallback(const CommandCallback& callback) {
   on_command_removed_.push_back(callback);
 }
 
+void CommandQueue::AddCommandHandler(const std::string& command_name,
+                                     const CommandCallback& callback) {
+  if (!command_name.empty()) {
+    CHECK(default_command_callback_.is_null())
+        << "Commands specific handler are not allowed after default one";
+
+    for (const auto& command : map_) {
+      if (command.second->GetStatus() == CommandStatus::kQueued &&
+          command.second->GetName() == command_name) {
+        callback.Run(command.second.get());
+      }
+    }
+
+    CHECK(command_callbacks_.emplace(command_name, callback).second)
+        << command_name << " already has handler";
+
+  } else {
+    for (const auto& command : map_) {
+      if (command.second->GetStatus() == CommandStatus::kQueued &&
+          command_callbacks_.find(command.second->GetName()) ==
+              command_callbacks_.end()) {
+        callback.Run(command.second.get());
+      }
+    }
+
+    CHECK(default_command_callback_.is_null()) << "Already has default handler";
+    default_command_callback_ = callback;
+  }
+}
+
 void CommandQueue::Add(std::unique_ptr<CommandInstance> instance) {
   std::string id = instance->GetID();
   LOG_IF(FATAL, id.empty()) << "Command has no ID";
@@ -35,6 +63,14 @@
                               << "' is already in the queue";
   for (const auto& cb : on_command_added_)
     cb.Run(pair.first->second.get());
+
+  auto it_handler = command_callbacks_.find(pair.first->second->GetName());
+
+  if (it_handler != command_callbacks_.end())
+    it_handler->second.Run(pair.first->second.get());
+  else if (!default_command_callback_.is_null())
+    default_command_callback_.Run(pair.first->second.get());
+
   Cleanup();
 }
 
diff --git a/libweave/src/commands/command_queue.h b/libweave/src/commands/command_queue.h
index c3e86d5..888c04f 100644
--- a/libweave/src/commands/command_queue.h
+++ b/libweave/src/commands/command_queue.h
@@ -25,11 +25,16 @@
  public:
   CommandQueue() = default;
 
+  using CommandCallback = Device::CommandHandlerCallback;
+
   // Adds notifications callback for a new command is added to the queue.
-  void AddCommandAddedCallback(const Device::CommandCallback& callback);
+  void AddCommandAddedCallback(const CommandCallback& callback);
 
   // Adds notifications callback for a command is removed from the queue.
-  void AddCommandRemovedCallback(const Device::CommandCallback& callback);
+  void AddCommandRemovedCallback(const CommandCallback& callback);
+
+  void AddCommandHandler(const std::string& command_name,
+                         const CommandCallback& callback);
 
   // Checks if the command queue is empty.
   bool IsEmpty() const { return map_.empty(); }
@@ -75,9 +80,11 @@
   // Queue of commands to be removed.
   std::queue<std::pair<base::Time, std::string>> remove_queue_;
 
-  using CallbackList = std::vector<Device::CommandCallback>;
+  using CallbackList = std::vector<CommandCallback>;
   CallbackList on_command_added_;
   CallbackList on_command_removed_;
+  std::map<std::string, CommandCallback> command_callbacks_;
+  CommandCallback default_command_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(CommandQueue);
 };
diff --git a/libweave/src/device_manager.cc b/libweave/src/device_manager.cc
index 6ff3a88..009df46 100644
--- a/libweave/src/device_manager.cc
+++ b/libweave/src/device_manager.cc
@@ -106,8 +106,9 @@
   return command_manager_->FindCommand(id);
 }
 
-void DeviceManager::AddCommandAddedCallback(const CommandCallback& callback) {
-  return command_manager_->AddCommandAddedCallback(callback);
+void DeviceManager::AddCommandHandler(const std::string& command_name,
+                                      const CommandHandlerCallback& callback) {
+  return command_manager_->AddCommandHandler(command_name, callback);
 }
 
 bool DeviceManager::SetStateProperties(
diff --git a/libweave/src/device_manager.h b/libweave/src/device_manager.h
index a72d800..72a24ab 100644
--- a/libweave/src/device_manager.h
+++ b/libweave/src/device_manager.h
@@ -41,7 +41,8 @@
                   std::string* id,
                   ErrorPtr* error) override;
   Command* FindCommand(const std::string& id) override;
-  void AddCommandAddedCallback(const CommandCallback& callback) override;
+  void AddCommandHandler(const std::string& command_name,
+                         const CommandHandlerCallback& callback) override;
   void AddStateChangedCallback(const base::Closure& callback) override;
   bool SetStateProperties(const base::DictionaryValue& property_set,
                           ErrorPtr* error) override;