Fix SDL does not unsubscribe from softbuttons after receiving a respo…

…nse. (#2629)

* Fix SDL does not unsubscribe from softbuttons after receiving a response.

When SDL recives AlertManeuverResponse, AlertResponse,
UpdateTurnListResponse, ScrollableMessageResponse, ShowConstantTBTResponse
SDL unsubscribes from soft buttons before sends response to mobile.
Added unit test cases for specified responses.

* Answer to PR comments.

* Answer to PR comments.

* Answer to PR comments.

* Apply typo fixes from code review

* Address comments and fix tests after merge

* Address comments

* Change keeping of softbuttons according to a WindowID

SDL should keep softbuttons subscriptions in pair with a WindowID that related
to the softbuttons

* Fix regression and style issues

* fixup! Fix regression and style issues

Co-authored-by: Jacob Keeler <jacob.keeler@livioradio.com>
Co-authored-by: Andrii Kalinich <AKalinich@luxoft.com>
Co-authored-by: Dmitriy Boltovskiy <dboltovskyi@luxoft.com>

src/components/application_manager/include/application_manager/application.h

@@ -170,9 +170,14 @@ typedef std::map<uint32_t, smart_objects::SmartObject*> PerformChoice;
 typedef std::map<uint32_t, PerformChoice> PerformChoiceSetMap;
 
 /**
- * @brief Defines id of SoftButton
+ * @brief Defines id of SoftButtons for specified WindowID
  */
-typedef std::set<std::pair<uint32_t, WindowID> > SoftButtonID;
+typedef std::pair<WindowID, std::set<uint32_t> > WindowSoftButtons;
+
+/**
+ * @brief Defines id of SoftButtons related to a specified WindowID
+ */
+typedef std::set<WindowSoftButtons> SoftButtonIDs;
 
 /**
  * @brief Defines set of buttons subscription
@@ -961,10 +966,10 @@ class Application : public virtual InitialApplicationData,
    * Alert, Show, ScrollableMessage, ShowConstantTBT, AlertManeuver,
    * UpdateTurnList
    * @param cmd_id Unique command id from mobile API
-   * @param list of softbuttons were created by command.
+   * @param window_softbuttons list of softbuttons were created by command.
    */
-  virtual void SubscribeToSoftButtons(int32_t cmd_id,
-                                      const SoftButtonID& softbuttons_id) = 0;
+  virtual void SubscribeToSoftButtons(
+      int32_t cmd_id, const WindowSoftButtons& window_softbuttons) = 0;
 
   /**
    * @brief Retreives window id on which given button is created

src/components/application_manager/include/application_manager/application_impl.h

@@ -279,8 +279,8 @@ class ApplicationImpl : public virtual Application,
 
   bool AreCommandLimitsExceeded(mobile_apis::FunctionID::eType cmd_id,
                                 TLimitSource source);
-  virtual void SubscribeToSoftButtons(int32_t cmd_id,
-                                      const SoftButtonID& softbuttons_id);
+  virtual void SubscribeToSoftButtons(
+      int32_t cmd_id, const WindowSoftButtons& window_softbuttons);
   virtual bool IsSubscribedToSoftButton(const uint32_t softbutton_id);
 
   virtual void UnsubscribeFromSoftButtons(int32_t cmd_id);
@@ -638,7 +638,7 @@ class ApplicationImpl : public virtual Application,
   /**
    * @brief Defines id of SoftButton which is related from name of command
    */
-  typedef std::map<int32_t, SoftButtonID> CommandSoftButtonID;
+  typedef std::map<int32_t, SoftButtonIDs> CommandSoftButtonID;
   CommandNumberTimeLimit cmd_number_to_time_limits_;
   CommandSoftButtonID cmd_softbuttonid_;
   // Lock for command soft button id

src/components/application_manager/include/application_manager/application_manager_impl.h

@@ -1172,6 +1172,9 @@ class ApplicationManagerImpl
   bool IsSOStructValid(const hmi_apis::StructIdentifiers::eType struct_id,
                        const smart_objects::SmartObject& display_capabilities);
 
+  virtual bool UnsubscribeAppFromSoftButtons(
+      const commands::MessageSharedPtr response) OVERRIDE;
+
   /**
    * @brief Function returns supported SDL Protocol Version
    * @return protocol version depends on parameters from smartDeviceLink.ini.

src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/alert_maneuver_response.cc

@@ -59,6 +59,8 @@ AlertManeuverResponse::~AlertManeuverResponse() {}
 void AlertManeuverResponse::Run() {
   SDL_LOG_AUTO_TRACE();
 
+  application_manager_.UnsubscribeAppFromSoftButtons(message_);
+
   rpc_service_.SendMessageToMobile(message_);
 }
 

src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/alert_response.cc

@@ -61,6 +61,8 @@ AlertResponse::~AlertResponse() {}
 void AlertResponse::Run() {
   SDL_LOG_AUTO_TRACE();
 
+  application_manager_.UnsubscribeAppFromSoftButtons(message_);
+
   rpc_service_.SendMessageToMobile(message_);
 }
 

src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/scrollable_message_response.cc

@@ -57,15 +57,7 @@ ScrollableMessageResponse::ScrollableMessageResponse(
 
 void ScrollableMessageResponse::Run() {
   SDL_LOG_AUTO_TRACE();
-  mobile_apis::Result::eType result_code =
-      static_cast<mobile_apis::Result::eType>(
-          (*message_)[strings::msg_params][strings::result_code].asInt());
-  ApplicationSharedPtr application = application_manager_.application(
-      (*message_)[strings::params][strings::connection_key].asInt());
-  if ((mobile_apis::Result::REJECTED != result_code) && application) {
-    application->UnsubscribeFromSoftButtons(
-        (*message_)[strings::params][strings::function_id].asInt());
-  }
+  application_manager_.UnsubscribeAppFromSoftButtons(message_);
   rpc_service_.SendMessageToMobile(message_);
 }
 

src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/show_constant_tbt_response.cc

@@ -60,6 +60,8 @@ ShowConstantTBTResponse::~ShowConstantTBTResponse() {}
 void ShowConstantTBTResponse::Run() {
   SDL_LOG_AUTO_TRACE();
 
+  application_manager_.UnsubscribeAppFromSoftButtons(message_);
+
   rpc_service_.SendMessageToMobile(message_);
 }
 

src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/update_turn_list_response.cc

@@ -59,6 +59,8 @@ UpdateTurnListResponse::~UpdateTurnListResponse() {}
 void UpdateTurnListResponse::Run() {
   SDL_LOG_AUTO_TRACE();
 
+  application_manager_.UnsubscribeAppFromSoftButtons(message_);
+
   rpc_service_.SendMessageToMobile(message_);
 }
 

src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/simple_response_commands_test.cc

@@ -93,15 +93,12 @@ class MobileResponseCommandsTest
 
 typedef Types<commands::ListFilesResponse,
               commands::DeleteCommandResponse,
-              commands::AlertManeuverResponse,
-              commands::AlertResponse,
               commands::SubscribeButtonResponse,
               commands::AddSubMenuResponse,
               commands::DialNumberResponse,
               commands::EndAudioPassThruResponse,
               commands::UnregisterAppInterfaceResponse,
               commands::UnsubscribeWayPointsResponse,
-              commands::UpdateTurnListResponse,
               commands::UnsubscribeButtonResponse,
               commands::SliderResponse,
               commands::SpeakResponse,
@@ -112,7 +109,6 @@ typedef Types<commands::ListFilesResponse,
               commands::PerformAudioPassThruResponse,
               commands::SetGlobalPropertiesResponse,
               commands::SetMediaClockTimerResponse,
-              commands::ShowConstantTBTResponse,
               commands::ShowResponse,
               commands::SystemResponse,
               commands::AddCommandResponse,
@@ -131,6 +127,35 @@ TYPED_TEST(MobileResponseCommandsTest, Run_SendResponseToMobile_SUCCESS) {
   command->Run();
 }
 
+template <class CommandWithUnsubscribe>
+class MobileResponseWithUnsubscribeCommandsTest
+    : public CommandsTest<CommandsTestMocks::kIsNice> {
+ public:
+  typedef CommandWithUnsubscribe UnsubscribeCommand;
+};
+
+typedef Types<commands::AlertManeuverResponse,
+              commands::AlertResponse,
+              commands::UpdateTurnListResponse,
+              commands::ScrollableMessageResponse,
+              commands::ShowConstantTBTResponse>
+    ResponseWithUnsubscribeCommandList;
+
+TYPED_TEST_CASE(MobileResponseWithUnsubscribeCommandsTest,
+                ResponseWithUnsubscribeCommandList);
+
+TYPED_TEST(MobileResponseWithUnsubscribeCommandsTest,
+           RunWithUnsubscribe_SUCCESS) {
+  std::shared_ptr<typename TestFixture::UnsubscribeCommand> command =
+      this->template CreateCommand<typename TestFixture::UnsubscribeCommand>();
+
+  EXPECT_CALL(this->app_mngr_, UnsubscribeAppFromSoftButtons(_));
+  EXPECT_CALL(this->mock_rpc_service_, SendMessageToMobile(NotNull(), _));
+
+  command->Init();
+  command->Run();
+}
+
 class GenericResponseFromHMICommandsTest
     : public CommandsTest<CommandsTestMocks::kIsNice> {};
 
@@ -145,7 +170,6 @@ MATCHER_P2(CheckMessageParams, success, result, "") {
       result ==
       static_cast<int32_t>(
           (*arg)[am::strings::msg_params][am::strings::result_code].asInt());
-
   using namespace helpers;
   return Compare<bool, EQ, ALL>(
       true, is_msg_type_correct, is_success_correct, is_result_code_correct);
@@ -164,24 +188,6 @@ TEST_F(GenericResponseFromHMICommandsTest, Run_SUCCESS) {
 
   command->Run();
 }
-
-class ScrollableMessageResponseTest
-    : public CommandsTest<CommandsTestMocks::kIsNice> {};
-
-TEST_F(ScrollableMessageResponseTest, Run_SUCCESS) {
-  MessageSharedPtr message = CreateMessage();
-  (*message)[am::strings::msg_params][am::strings::result_code] =
-      mobile_apis::Result::SUCCESS;
-
-  MockAppPtr app(CreateMockApp());
-
-  std::shared_ptr<commands::ScrollableMessageResponse> command(
-      CreateCommand<commands::ScrollableMessageResponse>(message));
-  EXPECT_CALL(app_mngr_, application(_)).WillOnce(Return(app));
-  EXPECT_CALL(*app, UnsubscribeFromSoftButtons(_));
-  command->Run();
-}
-
 }  // namespace simple_response_commands_test
 }  // namespace mobile_commands_test
 }  // namespace commands_test

src/components/application_manager/src/application_impl.cc

@@ -1147,20 +1147,36 @@ uint32_t ApplicationImpl::GetAvailableDiskSpace() {
 }
 
 void ApplicationImpl::SubscribeToSoftButtons(
-    int32_t cmd_id, const SoftButtonID& softbuttons_id) {
+    int32_t cmd_id, const WindowSoftButtons& window_softbuttons) {
   sync_primitives::AutoLock lock(cmd_softbuttonid_lock_);
-  if (static_cast<int32_t>(mobile_apis::FunctionID::ScrollableMessageID) ==
-      cmd_id) {
-    CommandSoftButtonID::iterator it = cmd_softbuttonid_.find(cmd_id);
-    if (cmd_softbuttonid_.end() == it) {
-      cmd_softbuttonid_[cmd_id] = softbuttons_id;
-    }
-  } else {
-    auto& soft_button_ids = cmd_softbuttonid_[cmd_id];
-    for (auto& softbutton_item : softbuttons_id) {
-      soft_button_ids.insert(softbutton_item);
-    }
+  CommandSoftButtonID::iterator it = cmd_softbuttonid_.find(cmd_id);
+
+  if (cmd_softbuttonid_.end() == it) {
+    SoftButtonIDs soft_buttons{window_softbuttons};
+    cmd_softbuttonid_.insert({cmd_id, soft_buttons});
+    return;
   }
+
+  auto& command_soft_buttons = cmd_softbuttonid_[cmd_id];
+
+  const auto window_id = window_softbuttons.first;
+  auto find_window_id = [window_id](const WindowSoftButtons& window_buttons) {
+    return window_id == window_buttons.first;
+  };
+
+  auto subscribed_window_buttons = std::find_if(
+      command_soft_buttons.begin(), command_soft_buttons.end(), find_window_id);
+
+  if (subscribed_window_buttons == command_soft_buttons.end()) {
+    command_soft_buttons.insert(window_softbuttons);
+    return;
+  }
+
+  WindowSoftButtons new_window_soft_buttons = *subscribed_window_buttons;
+  new_window_soft_buttons.second.insert(window_softbuttons.second.begin(),
+                                        window_softbuttons.second.end());
+  command_soft_buttons.erase(subscribed_window_buttons);
+  command_soft_buttons.insert(new_window_soft_buttons);
 }
 
 struct FindSoftButtonId {
@@ -1169,39 +1185,65 @@ struct FindSoftButtonId {
   explicit FindSoftButtonId(const uint32_t soft_button_id)
       : soft_button_id_(soft_button_id) {}
 
-  bool operator()(const std::pair<uint32_t, WindowID>& element) const {
-    return soft_button_id_ == element.first;
+  bool operator()(const uint32_t softbutton_id) {
+    return soft_button_id_ == softbutton_id;
   }
 };
 
+struct FindWindowSoftButtonId {
+ public:
+  FindWindowSoftButtonId(const uint32_t soft_button_id)
+      : find_softbutton_id_(soft_button_id) {}
+
+  bool operator()(const WindowSoftButtons& window_buttons) {
+    const auto softbuttons = window_buttons.second;
+    auto found_softbutton = std::find_if(
+        softbuttons.begin(), softbuttons.end(), find_softbutton_id_);
+
+    return found_softbutton != softbuttons.end();
+  }
+
+ private:
+  FindSoftButtonId find_softbutton_id_;
+};
+
 bool ApplicationImpl::IsSubscribedToSoftButton(const uint32_t softbutton_id) {
   SDL_LOG_AUTO_TRACE();
   sync_primitives::AutoLock lock(cmd_softbuttonid_lock_);
-  CommandSoftButtonID::iterator it = cmd_softbuttonid_.begin();
-  for (; it != cmd_softbuttonid_.end(); ++it) {
-    const auto& soft_button_ids = (*it).second;
-    FindSoftButtonId finder(softbutton_id);
-    const auto find_res =
-        std::find_if(soft_button_ids.begin(), soft_button_ids.end(), finder);
-    if ((soft_button_ids.end() != find_res)) {
+
+  for (const auto& command_soft_buttons : cmd_softbuttonid_) {
+    FindWindowSoftButtonId find_window_softbutton_id(softbutton_id);
+    const auto& window_softbuttons = command_soft_buttons.second;
+
+    const auto found_window_softbutton_id =
+        std::find_if(window_softbuttons.begin(),
+                     window_softbuttons.end(),
+                     find_window_softbutton_id);
+
+    if (found_window_softbutton_id != window_softbuttons.end()) {
       return true;
     }
   }
+
   return false;
 }
 
 WindowID ApplicationImpl::GetSoftButtonWindowID(const uint32_t softbutton_id) {
   SDL_LOG_AUTO_TRACE();
 
   sync_primitives::AutoLock lock(cmd_softbuttonid_lock_);
-  CommandSoftButtonID::iterator it = cmd_softbuttonid_.begin();
-  for (; it != cmd_softbuttonid_.end(); ++it) {
-    const auto& soft_button_ids = (*it).second;
-    FindSoftButtonId finder(softbutton_id);
-    const auto find_res =
-        std::find_if(soft_button_ids.begin(), soft_button_ids.end(), finder);
-    if ((soft_button_ids.end() != find_res)) {
-      return find_res->second;
+
+  for (const auto& command_soft_buttons : cmd_softbuttonid_) {
+    FindWindowSoftButtonId find_window_softbutton_id(softbutton_id);
+    const auto& window_softbuttons = command_soft_buttons.second;
+
+    const auto found_window_softbutton_id =
+        std::find_if(window_softbuttons.begin(),
+                     window_softbuttons.end(),
+                     find_window_softbutton_id);
+
+    if (found_window_softbutton_id != window_softbuttons.end()) {
+      return found_window_softbutton_id->first;
     }
   }
 

src/components/application_manager/src/application_manager_impl.cc

@@ -4898,6 +4898,23 @@ bool ApplicationManagerImpl::IsSOStructValid(
   return false;
 }
 
+bool ApplicationManagerImpl::UnsubscribeAppFromSoftButtons(
+    const commands::MessageSharedPtr response) {
+  using namespace mobile_apis;
+
+  const uint32_t connection_key =
+      (*response)[strings::params][strings::connection_key].asUInt();
+  const auto function_id = static_cast<FunctionID::eType>(
+      (*response)[strings::params][strings::function_id].asInt());
+
+  ApplicationSharedPtr app = application(connection_key);
+  DCHECK_OR_RETURN(app, false);
+  app->UnsubscribeFromSoftButtons(function_id);
+  SDL_LOG_DEBUG("Application has unsubscribed from softbuttons. FunctionID: "
+                << function_id << ", app_id:" << app->app_id());
+  return true;
+}
+
 #ifdef BUILD_TESTS
 void ApplicationManagerImpl::AddMockApplication(ApplicationSharedPtr mock_app) {
   applications_list_lock_ptr_->Acquire();

src/components/application_manager/src/message_helper/message_helper.cc

@@ -3365,16 +3365,20 @@ void MessageHelper::SubscribeApplicationToSoftButton(
     ApplicationSharedPtr app,
     int32_t function_id,
     const WindowID window_id) {
-  SoftButtonID softbuttons_id;
-  smart_objects::SmartObject& soft_buttons =
-      message_params[strings::soft_buttons];
-  unsigned int length = soft_buttons.length();
-  for (unsigned int i = 0; i < length; ++i) {
-    const auto button_id = std::make_pair(
-        soft_buttons[i][strings::soft_button_id].asUInt(), window_id);
-    softbuttons_id.insert(button_id);
-  }
-  app->SubscribeToSoftButtons(function_id, softbuttons_id);
+  if (!message_params.keyExists(strings::soft_buttons)) {
+    return;
+  }
+
+  std::set<uint32_t> soft_buttons;
+
+  auto& soft_buttons_so = message_params[strings::soft_buttons];
+  for (const auto& softbutton : *(soft_buttons_so.asArray())) {
+    const auto button_id = softbutton[strings::soft_button_id].asUInt();
+    soft_buttons.insert(button_id);
+  }
+
+  WindowSoftButtons window_buttons{window_id, soft_buttons};
+  app->SubscribeToSoftButtons(function_id, window_buttons);
 }
 
 void MessageHelper::SubscribeApplicationToSoftButton(