Fixes RPCs holding for applications being switched

6 files changed, 221 insertions, 55 deletions

diff --git a/src/components/application_manager/include/application_manager/command_holder.h b/src/components/application_manager/include/application_manager/command_holder.h
index 02ae91e30c..4023f184b0 100644
--- a/src/components/application_manager/include/application_manager/command_holder.h
+++ b/src/components/application_manager/include/application_manager/command_holder.h
@@ -33,17 +33,23 @@
 #pragma once
 
 #include <string>
+#include "application_manager/application.h"
 #include "smart_objects/smart_object.h"
 #include "utils/shared_ptr.h"
 
 namespace application_manager {
 /**
  * @brief The CommandHolder class should hold commands for particular
- * application specified by its id
+ * application until certain event happens
  */
 class CommandHolder {
  public:
   /**
+   * @brief The CommandType enum defines type of command to suspend or resume
+   */
+  enum class CommandType { kHmiCommand, kMobileCommand };
+
+  /**
    * @brief ~CommandsHolder destructor
    */
   virtual ~CommandHolder() {}
@@ -51,23 +57,26 @@ class CommandHolder {
   /**
    * @brief Suspend collects command for specific application policy id
    * internally
-   * @param policy_app_id Application policy id
+   * @param application Application pointer
+   * @param type Command type
    * @param command Command
    */
-  virtual void Suspend(const std::string& policy_app_id,
+  virtual void Suspend(ApplicationSharedPtr application,
+                       CommandType type,
                        smart_objects::SmartObjectSPtr command) = 0;
 
   /**
    * @brief Resume send all collected commands for further processing and
    * removes them afterward
-   * @param policy_app_id Application policy id
+   * @param application Application pointer
+   * @param type Command type
    */
-  virtual void Resume(const std::string& policy_app_id) = 0;
+  virtual void Resume(ApplicationSharedPtr application, CommandType type) = 0;
 
   /**
    * @brief Clear removes all collected commands w/o processing
-   * @param policy_app_id Application policy id
+   * @param application Application pointer
    */
-  virtual void Clear(const std::string& policy_app_id) = 0;
+  virtual void Clear(ApplicationSharedPtr application) = 0;
 };
 }  // application_manager
diff --git a/src/components/application_manager/include/application_manager/command_holder_impl.h b/src/components/application_manager/include/application_manager/command_holder_impl.h
index a9dfa46bc7..a519f310c3 100644
--- a/src/components/application_manager/include/application_manager/command_holder_impl.h
+++ b/src/components/application_manager/include/application_manager/command_holder_impl.h
@@ -36,6 +36,8 @@
 
 #include <string>
 #include <vector>
+#include <map>
+#include "application_manager/application.h"
 #include "smart_objects/smart_object.h"
 #include "utils/lock.h"
 #include "utils/shared_ptr.h"
@@ -44,10 +46,9 @@
 namespace application_manager {
 class ApplicationManager;
 /**
- * @brief The CommandHolderImpl class should hold HMI commands for particular
- * application specified by its policy id during application transport switching
- * process and sends for processing after switching is completed successfully
- * or drop otherwise
+ * @brief The CommandHolderImpl class should hold commands for particular
+ * application during application transport switching process and sends for
+ * processing after switching is completed successfully or drops otherwise
  */
 class CommandHolderImpl : public CommandHolder {
  public:
@@ -59,32 +60,48 @@ class CommandHolderImpl : public CommandHolder {
 
   /**
    * @brief Suspend collects command for specific application id internally
-   * @param policy_app_id Policy id of application
+   * @param application Application pointer
+   * @param type Command type
    * @param command Command
    */
-  void Suspend(const std::string& policy_app_id,
+  void Suspend(ApplicationSharedPtr application,
+               CommandType type,
                smart_objects::SmartObjectSPtr command) FINAL;
 
   /**
    * @brief Resume sends all collected HMI commands to ApplicationManager
    * for further processing
-   * @param policy_app_id Policy id of application
+   * @param application Application pointer
+   * @param type Command type
    */
-  void Resume(const std::string& policy_app_id) FINAL;
+  void Resume(ApplicationSharedPtr application, CommandType type) FINAL;
 
   /**
    * @brief Clear removes all commands collected for specific application id
-   * @param policy_app_id Policy application id
+   * @param application Application pointer
    */
-  void Clear(const std::string& policy_app_id) FINAL;
+  void Clear(ApplicationSharedPtr application) FINAL;
 
  private:
+  /**
+   * @brief ResumeHmiCommand sends suspended HMI commands for processing
+   * @param application Application which commands to process
+   */
+  void ResumeHmiCommand(ApplicationSharedPtr app);
+
+  /**
+   * @brief ResumeMobileCommand sends suspended mobile commands for processing
+   * @param application Application which commands to process
+   */
+  void ResumeMobileCommand(ApplicationSharedPtr application);
+
   using AppCommands =
-      std::map<std::string,
+      std::map<ApplicationSharedPtr,
                std::vector<utils::SharedPtr<smart_objects::SmartObject> > >;
 
   ApplicationManager& app_manager_;
   sync_primitives::Lock commands_lock_;
-  AppCommands app_commands_;
+  AppCommands app_mobile_commands_;
+  AppCommands app_hmi_commands_;
 };
 }  // application_manager
diff --git a/src/components/application_manager/src/application_manager_impl.cc b/src/components/application_manager/src/application_manager_impl.cc
index 03e9a7d95e..f9fa396d80 100644
--- a/src/components/application_manager/src/application_manager_impl.cc
+++ b/src/components/application_manager/src/application_manager_impl.cc
@@ -391,7 +391,9 @@ void ApplicationManagerImpl::OnApplicationRegistered(ApplicationSharedPtr app) {
 }
 
 void ApplicationManagerImpl::OnApplicationSwitched(ApplicationSharedPtr app) {
-  commands_holder_->Resume(app->policy_app_id());
+  LOG4CXX_AUTO_TRACE(logger_);
+  commands_holder_->Resume(app, CommandHolder::CommandType::kMobileCommand);
+  commands_holder_->Resume(app, CommandHolder::CommandType::kHmiCommand);
 }
 
 bool ApplicationManagerImpl::IsAppTypeExistsInFullOrLimited(
@@ -1102,8 +1104,7 @@ void ApplicationManagerImpl::OnDeviceSwitchingFinish(
   for (auto app_it = reregister_wait_list_.begin();
        app_it != reregister_wait_list_.end();
        ++app_it) {
-    auto app = *app_it;
-    commands_holder_->Clear(app->policy_app_id());
+    auto app = *app_it;    
     UnregisterApplication(app->app_id(),
                           mobile_apis::Result::INVALID_ENUM,
                           is_resuming,
@@ -1823,6 +1824,17 @@ bool ApplicationManagerImpl::ManageMobileCommand(
     return false;
   }
 
+  const uint32_t connection_key = static_cast<uint32_t>(
+      (*message)[strings::params][strings::connection_key].asUInt());
+
+  auto app_ptr = application(connection_key);
+  if (app_ptr && IsAppInReconnectMode(app_ptr->policy_app_id())) {
+    commands_holder_->Suspend(app_ptr,
+                              CommandHolder::CommandType::kMobileCommand,
+                              message);
+    return true;
+  }
+
   mobile_apis::FunctionID::eType function_id =
       static_cast<mobile_apis::FunctionID::eType>(
           (*message)[strings::params][strings::function_id].asInt());
@@ -1833,9 +1845,6 @@ bool ApplicationManagerImpl::ManageMobileCommand(
           ? (*message)[strings::params][strings::correlation_id].asUInt()
           : 0;
 
-  uint32_t connection_key =
-      (*message)[strings::params][strings::connection_key].asUInt();
-
   int32_t protocol_type =
       (*message)[strings::params][strings::protocol_type].asUInt();
 
@@ -2052,10 +2061,18 @@ bool ApplicationManagerImpl::ManageHMICommand(
     return false;
   }
 
-  auto app = application(command->connection_key());
-  if (app && IsAppInReconnectMode(app->policy_app_id())) {
-    commands_holder_->Suspend(app->policy_app_id(), message);
-    return true;
+  if ((*message).keyExists(strings::msg_params) &&
+      (*message)[strings::msg_params].keyExists(strings::app_id)) {
+    const auto connection_key =
+        (*message)[strings::msg_params][strings::app_id].asUInt();
+
+    auto app = application(static_cast<uint32_t>(connection_key));
+    if (app && IsAppInReconnectMode(app->policy_app_id())) {
+      commands_holder_->Suspend(app,
+                                CommandHolder::CommandType::kHmiCommand,
+                                message);
+      return true;
+    }
   }
 
   int32_t message_type =
@@ -3123,6 +3140,9 @@ void ApplicationManagerImpl::UnregisterApplication(
       SendUpdateAppList();
     }
   }
+
+  commands_holder_->Clear(app_to_remove);
+
   if (audio_pass_thru_active_) {
     // May be better to put this code in MessageHelper?
     EndAudioPassThrough();
diff --git a/src/components/application_manager/src/command_holder_impl.cc b/src/components/application_manager/src/command_holder_impl.cc
index dfb100c802..d5700f512e 100644
--- a/src/components/application_manager/src/command_holder_impl.cc
+++ b/src/components/application_manager/src/command_holder_impl.cc
@@ -32,38 +32,102 @@
 
 #include "application_manager/command_holder_impl.h"
 #include "application_manager/application_manager.h"
+#include "application_manager/commands/command.h"
 
 namespace application_manager {
+CREATE_LOGGERPTR_GLOBAL(logger_, "ApplicationManager")
+
 CommandHolderImpl::CommandHolderImpl(ApplicationManager& app_manager)
     : app_manager_(app_manager) {}
 
 void CommandHolderImpl::Suspend(
-    const std::string& policy_app_id,
+    ApplicationSharedPtr application,
+    CommandType type,
     utils::SharedPtr<smart_objects::SmartObject> command) {
+  LOG4CXX_AUTO_TRACE(logger_);
+  LOG4CXX_DEBUG(logger_, "Suspending command(s) for application: "
+                << application->policy_app_id());
+  sync_primitives::AutoLock lock(commands_lock_);
+
+  if (CommandType::kHmiCommand == type) {
+    app_hmi_commands_[application].push_back(command);
+    LOG4CXX_DEBUG(logger_, "Suspended HMI command(s): "
+                  << app_hmi_commands_.size());
+  } else {
+    app_mobile_commands_[application].push_back(command);
+    LOG4CXX_DEBUG(logger_, "Suspended mobile command(s): "
+                  << app_hmi_commands_.size());
+  }
+}
+
+void CommandHolderImpl::Resume(ApplicationSharedPtr application,
+                               CommandType type) {
+  LOG4CXX_AUTO_TRACE(logger_);
+  LOG4CXX_DEBUG(logger_, "Resuming command(s) for application: "
+                << application->policy_app_id());
+  if (CommandType::kHmiCommand == type) {
+    ResumeHmiCommand(application);
+  } else {
+    ResumeMobileCommand(application);
+  }
+}
+
+void CommandHolderImpl::Clear(ApplicationSharedPtr application) {
+  LOG4CXX_AUTO_TRACE(logger_);
+  LOG4CXX_DEBUG(logger_, "Clearing command(s) for application: "
+                << application->policy_app_id());
   sync_primitives::AutoLock lock(commands_lock_);
-  app_commands_[policy_app_id].push_back(command);
+  auto app_hmi_commands = app_hmi_commands_.find(application);
+  if (app_hmi_commands_.end() != app_hmi_commands) {
+    LOG4CXX_DEBUG(logger_, "Clearing HMI command(s): "
+                  << app_hmi_commands->second.size());
+    app_hmi_commands_.erase(app_hmi_commands);
+  }
+
+  auto app_mobile_commands = app_mobile_commands_.find(application);
+  if (app_mobile_commands_.end() != app_mobile_commands) {
+    LOG4CXX_DEBUG(logger_, "Clearing mobile command(s): "
+                  << app_mobile_commands->second.size());
+    app_mobile_commands_.erase(app_mobile_commands);
+  }
 }
 
-void CommandHolderImpl::Resume(const std::string& policy_app_id) {
+void CommandHolderImpl::ResumeHmiCommand(ApplicationSharedPtr application) {
   sync_primitives::AutoLock lock(commands_lock_);
-  auto app_commands = app_commands_.find(policy_app_id);
-  if (app_commands_.end() == app_commands) {
+  auto app_commands = app_hmi_commands_.find(application);
+  if (app_hmi_commands_.end() == app_commands) {
     return;
   }
+
+  LOG4CXX_DEBUG(logger_, "Resuming HMI command(s): "
+                << app_hmi_commands_.size());
+
   for (auto cmd : app_commands->second) {
+    (*cmd)[strings::msg_params][strings::app_id] = application->hmi_app_id();
     app_manager_.ManageHMICommand(cmd);
   }
 
-  app_commands_.erase(app_commands);
+  app_hmi_commands_.erase(app_commands);
 }
 
-void CommandHolderImpl::Clear(const std::string& policy_app_id) {
+void CommandHolderImpl::ResumeMobileCommand(ApplicationSharedPtr application) {
   sync_primitives::AutoLock lock(commands_lock_);
-  auto app_commands = app_commands_.find(policy_app_id);
-  if (app_commands_.end() == app_commands) {
+  auto app_commands = app_mobile_commands_.find(application);
+  if (app_mobile_commands_.end() == app_commands) {
     return;
   }
 
-  app_commands_.erase(app_commands);
+  LOG4CXX_DEBUG(logger_, "Resuming mobile command(s): "
+                << app_mobile_commands_.size());
+
+  for (auto cmd : app_commands->second) {
+    (*cmd)[strings::params][strings::connection_key] =
+        application->app_id();
+    app_manager_.ManageMobileCommand(
+          cmd,
+          commands::Command::CommandOrigin::ORIGIN_MOBILE);
+  }
+
+  app_mobile_commands_.erase(app_commands);
 }
 }  // application_manager
diff --git a/src/components/application_manager/test/application_manager_impl_mock_hmi_test.cc b/src/components/application_manager/test/application_manager_impl_mock_hmi_test.cc
index 7ca2ff9d98..4de7be7c4c 100644
--- a/src/components/application_manager/test/application_manager_impl_mock_hmi_test.cc
+++ b/src/components/application_manager/test/application_manager_impl_mock_hmi_test.cc
@@ -202,6 +202,10 @@ TEST_F(ApplicationManagerImplMockHmiTest,
   commands::MessageSharedPtr hmi_msg_3 =
       utils::MakeShared<smart_objects::SmartObject>();
 
+  (*hmi_msg_1)[strings::msg_params][strings::app_id] =
+  (*hmi_msg_2)[strings::msg_params][strings::app_id] =
+  (*hmi_msg_3)[strings::msg_params][strings::app_id] = application_id;
+
   EXPECT_CALL(*cmd_1, Init()).Times(0);
   EXPECT_CALL(*cmd_2, Init()).Times(0);
   EXPECT_CALL(*cmd_3, Init()).Times(0);
diff --git a/src/components/application_manager/test/command_holder_test.cc b/src/components/application_manager/test/command_holder_test.cc
index 8082c3811f..e10cd5d008 100644
--- a/src/components/application_manager/test/command_holder_test.cc
+++ b/src/components/application_manager/test/command_holder_test.cc
@@ -33,36 +33,56 @@
 #include <gmock/gmock.h>
 
 #include "application_manager/command_holder_impl.h"
+#include "application_manager/commands/command.h"
 #include "smart_objects/smart_object.h"
 #include "utils/shared_ptr.h"
+#include "utils/make_shared.h"
 
 #include "application_manager/mock_application_manager.h"
+#include "application_manager/mock_application.h"
 
 namespace test {
 namespace components {
 namespace application_manager_test {
 
 using testing::_;
+using testing::Return;
 
 namespace am = application_manager;
 
 class CommandHolderImplTest : public testing::Test {
  public:
   CommandHolderImplTest()
-      : cmd_ptr_(new smart_objects::SmartObject), kPolicyAppId_("p_app_id") {}
+      : kPolicyAppId_("p_app_id")
+      , kHmiApplicationId_(123)
+      , kConnectionKey_(56789)
+      , cmd_ptr_(new smart_objects::SmartObject)
+      , mock_app_ptr_(new MockApplication) {}
+
+  void SetUp() OVERRIDE {
+    ON_CALL(*mock_app_ptr_, app_id()).WillByDefault(Return(kConnectionKey_));
+    ON_CALL(*mock_app_ptr_, hmi_app_id())
+        .WillByDefault(Return(kHmiApplicationId_));
+    ON_CALL(*mock_app_ptr_, policy_app_id())
+        .WillByDefault(Return(kPolicyAppId_));
+  }
 
   MockApplicationManager mock_app_manager_;
-  utils::SharedPtr<smart_objects::SmartObject> cmd_ptr_;
   const std::string kPolicyAppId_;
+  const uint32_t kHmiApplicationId_;
+  const uint32_t kConnectionKey_;
+  utils::SharedPtr<smart_objects::SmartObject> cmd_ptr_;
+  utils::SharedPtr<MockApplication> mock_app_ptr_;
 };
 
 TEST_F(CommandHolderImplTest, HoldOne_ExpectReleaseOne) {
   am::CommandHolderImpl cmd_holder(mock_app_manager_);
-  cmd_holder.Suspend(kPolicyAppId_, cmd_ptr_);
+  cmd_holder.Suspend(
+      mock_app_ptr_, am::CommandHolder::CommandType::kHmiCommand, cmd_ptr_);
 
   // Act
   EXPECT_CALL(mock_app_manager_, ManageHMICommand(cmd_ptr_));
-  cmd_holder.Resume(kPolicyAppId_);
+  cmd_holder.Resume(mock_app_ptr_, am::CommandHolder::CommandType::kHmiCommand);
 }
 
 TEST_F(CommandHolderImplTest, HoldMany_ExpectReleaseSame) {
@@ -70,34 +90,42 @@ TEST_F(CommandHolderImplTest, HoldMany_ExpectReleaseSame) {
 
   int32_t iterations = 0;
   do {
-    cmd_holder.Suspend(kPolicyAppId_, cmd_ptr_);
+    cmd_holder.Suspend(
+        mock_app_ptr_, am::CommandHolder::CommandType::kHmiCommand, cmd_ptr_);
     ++iterations;
   } while (iterations < 5);
 
   // Act
   EXPECT_CALL(mock_app_manager_, ManageHMICommand(cmd_ptr_)).Times(iterations);
-  cmd_holder.Resume(kPolicyAppId_);
+  cmd_holder.Resume(mock_app_ptr_, am::CommandHolder::CommandType::kHmiCommand);
 }
 
 TEST_F(CommandHolderImplTest, Hold_Drop_ExpectNoReleased) {
   am::CommandHolderImpl cmd_holder(mock_app_manager_);
-  cmd_holder.Suspend(kPolicyAppId_, cmd_ptr_);
-  cmd_holder.Suspend(kPolicyAppId_, cmd_ptr_);
+  cmd_holder.Suspend(
+      mock_app_ptr_, am::CommandHolder::CommandType::kHmiCommand, cmd_ptr_);
+  cmd_holder.Suspend(
+      mock_app_ptr_, am::CommandHolder::CommandType::kHmiCommand, cmd_ptr_);
 
   // Act
-  cmd_holder.Clear(kPolicyAppId_);
+  cmd_holder.Clear(mock_app_ptr_);
   EXPECT_CALL(mock_app_manager_, ManageHMICommand(cmd_ptr_)).Times(0);
-  cmd_holder.Resume(kPolicyAppId_);
+  cmd_holder.Resume(mock_app_ptr_, am::CommandHolder::CommandType::kHmiCommand);
 }
 
 TEST_F(CommandHolderImplTest, Hold_ReleaseAnotherId_ExpectNoReleased) {
   am::CommandHolderImpl cmd_holder(mock_app_manager_);
-  cmd_holder.Suspend(kPolicyAppId_, cmd_ptr_);
-  cmd_holder.Suspend(kPolicyAppId_, cmd_ptr_);
+  cmd_holder.Suspend(
+      mock_app_ptr_, am::CommandHolder::CommandType::kHmiCommand, cmd_ptr_);
+  cmd_holder.Suspend(
+      mock_app_ptr_, am::CommandHolder::CommandType::kHmiCommand, cmd_ptr_);
 
   // Act
+  utils::SharedPtr<MockApplication> another_app =
+      utils::MakeShared<MockApplication>();
+
   EXPECT_CALL(mock_app_manager_, ManageHMICommand(cmd_ptr_)).Times(0);
-  cmd_holder.Resume("anotherId");
+  cmd_holder.Resume(another_app, am::CommandHolder::CommandType::kHmiCommand);
 }
 
 TEST_F(CommandHolderImplTest, Hold_DropAnotherId_ExpectReleased) {
@@ -105,15 +133,39 @@ TEST_F(CommandHolderImplTest, Hold_DropAnotherId_ExpectReleased) {
 
   int32_t iterations = 0;
   do {
-    cmd_holder.Suspend(kPolicyAppId_, cmd_ptr_);
+    cmd_holder.Suspend(
+        mock_app_ptr_, am::CommandHolder::CommandType::kHmiCommand, cmd_ptr_);
     ++iterations;
   } while (iterations < 3);
 
   // Act
-  cmd_holder.Clear("anotherId");
+  utils::SharedPtr<MockApplication> another_app =
+      utils::MakeShared<MockApplication>();
+  cmd_holder.Clear(another_app);
 
   EXPECT_CALL(mock_app_manager_, ManageHMICommand(cmd_ptr_)).Times(iterations);
-  cmd_holder.Resume(kPolicyAppId_);
+  cmd_holder.Resume(mock_app_ptr_, am::CommandHolder::CommandType::kHmiCommand);
+}
+
+TEST_F(CommandHolderImplTest, Hold_Mobile_and_HMI_commands_ExpectReleased) {
+  am::CommandHolderImpl cmd_holder(mock_app_manager_);
+
+  cmd_holder.Suspend(
+      mock_app_ptr_, am::CommandHolder::CommandType::kHmiCommand, cmd_ptr_);
+
+  cmd_holder.Suspend(
+      mock_app_ptr_, am::CommandHolder::CommandType::kMobileCommand, cmd_ptr_);
+
+  // Act
+  EXPECT_CALL(mock_app_manager_, ManageHMICommand(cmd_ptr_));
+  cmd_holder.Resume(mock_app_ptr_, am::CommandHolder::CommandType::kHmiCommand);
+
+  EXPECT_CALL(
+      mock_app_manager_,
+      ManageMobileCommand(cmd_ptr_,
+                          am::commands::Command::CommandOrigin::ORIGIN_MOBILE));
+  cmd_holder.Resume(mock_app_ptr_,
+                    am::CommandHolder::CommandType::kMobileCommand);
 }
 
 }  // application_manager_test