commit 2033433d222c8c8aa56c94660e8e823dac71f45d
author Alex Vakulenko <avakulenko@google.com> Tue Dec 08 13:19:49 2015 -0800
committer Alex Vakulenko <avakulenko@google.com> Tue Dec 08 22:28:02 2015 +0000
tree 4a6beb3f139457be2a7f8bd226e5dd75ab13fbfc
parent f1fa8be44721c3e37e835a0dbd66e777200daf52

Fix a crash when adding a default command handler

It is allowed to call ComponentManager::AddCommandHandler with
empty component and command name to indicate that we are installing
a default command handler. The CHECK() condition should be only
verified if a specific command handler is specified.

Change-Id: Ib4c0f633e5e4ddc396a4f5030a6ebd4e411f911e
Reviewed-on: https://weave-review.googlesource.com/1820
Reviewed-by: Vitaly Buka <vitalybuka@google.com>

src/component_manager_impl.cc

diff --git a/src/component_manager_impl.cc b/src/component_manager_impl.cc
index 8f685b3..a23f34d 100644
--- a/src/component_manager_impl.cc
+++ b/src/component_manager_impl.cc
@@ -266,8 +266,12 @@
     const std::string& component_path,
     const std::string& command_name,
     const Device::CommandHandlerCallback& callback) {
-  CHECK(FindCommandDefinition(command_name))
-      << "Command undefined: " << command_name;
+  // If both component_path and command_name are empty, we are adding the
+  // default handler for all commands.
+  if (!component_path.empty() || !command_name.empty()) {
+    CHECK(FindCommandDefinition(command_name))
+        << "Command undefined: " << command_name;
+  }
   command_queue_.AddCommandHandler(component_path, command_name, callback);
 }
 

src/component_manager_unittest.cc

diff --git a/src/component_manager_unittest.cc b/src/component_manager_unittest.cc
index b35e16a..902ee15 100644
--- a/src/component_manager_unittest.cc
+++ b/src/component_manager_unittest.cc
@@ -819,6 +819,55 @@
   last_tags.clear();
 }
 
+TEST(ComponentManager, AddDefaultCommandHandler) {
+  ComponentManagerImpl manager;
+  const char kTraits[] = R"({
+    "trait1": {
+      "commands": {
+        "command1": { "minimalRole": "user" }
+      }
+    },
+    "trait2": {
+      "commands": {
+        "command2": { "minimalRole": "user" }
+      }
+    }
+  })";
+  auto traits = CreateDictionaryValue(kTraits);
+  ASSERT_TRUE(manager.LoadTraits(*traits, nullptr));
+  ASSERT_TRUE(manager.AddComponent("", "comp", {"trait1", "trait2"}, nullptr));
+
+  int count = 0;
+  auto handler = [&count](int tag, const std::weak_ptr<Command>& command) {
+    count++;
+  };
+
+  manager.AddCommandHandler("", "", base::Bind(handler, 1));
+  EXPECT_EQ(0, count);
+
+  const char kCommand1[] = R"({
+    "name": "trait1.command1",
+    "component": "comp"
+  })";
+  auto command1 = CreateDictionaryValue(kCommand1);
+  auto command_instance = manager.ParseCommandInstance(
+      *command1, Command::Origin::kCloud, UserRole::kUser, nullptr, nullptr);
+  ASSERT_NE(nullptr, command_instance.get());
+  manager.AddCommand(std::move(command_instance));
+  EXPECT_EQ(1, count);
+
+  const char kCommand2[] = R"({
+    "name": "trait2.command2",
+    "component": "comp"
+  })";
+  auto command2 = CreateDictionaryValue(kCommand2);
+  command_instance = manager.ParseCommandInstance(
+      *command2, Command::Origin::kCloud, UserRole::kUser, nullptr, nullptr);
+  ASSERT_NE(nullptr, command_instance.get());
+  manager.AddCommand(std::move(command_instance));
+  EXPECT_EQ(2, count);
+}
+
 TEST(ComponentManager, SetStateProperties) {
   ComponentManagerImpl manager;
   CreateTestComponentTree(&manager);