Delete both successful and pending commands on AddComand failure (#3940)

* Delete both successful and pending commands on AddComand failure

* Fix unit tests

2 files changed, 49 insertions, 36 deletions

diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/add_command_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/add_command_request.cc
index ddd2c9db35..eaad84dafd 100644
--- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/add_command_request.cc
+++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/add_command_request.cc
@@ -444,12 +444,6 @@ void AddCommandRequest::on_event(const event_engine::Event& event) {
       result_code = ui_result_ == hmi_apis::Common_Result::REJECTED
                         ? mobile_apis::Result::REJECTED
                         : mobile_apis::Result::GENERIC_ERROR;
-
-      msg_params[strings::grammar_id] = application->get_grammar_id();
-      msg_params[strings::type] = hmi_apis::Common_VRCommandType::Command;
-
-      SendHMIRequest(hmi_apis::FunctionID::VR_DeleteCommand, &msg_params);
-      application->RemoveCommand(cmd_id);
       result = false;
       SDL_LOG_DEBUG("Result " << result);
     }
@@ -461,10 +455,6 @@ void AddCommandRequest::on_event(const event_engine::Event& event) {
     result_code = vr_result_ == hmi_apis::Common_Result::REJECTED
                       ? mobile_apis::Result::REJECTED
                       : mobile_apis::Result::GENERIC_ERROR;
-
-    SendHMIRequest(hmi_apis::FunctionID::UI_DeleteCommand, &msg_params);
-
-    application->RemoveCommand(cmd_id);
     result = false;
     SDL_LOG_DEBUG("Result " << result);
   }
@@ -588,12 +578,28 @@ const std::string AddCommandRequest::GenerateMobileResponseInfo() {
 
 void AddCommandRequest::RemoveCommand() {
   SDL_LOG_AUTO_TRACE();
+  using namespace helpers;
+
   ApplicationSharedPtr app = application_manager_.application(connection_key());
   if (app.use_count() == 0) {
     SDL_LOG_ERROR("No application associated with session key");
     return;
   }
 
+  const bool is_ui_result_ok_or_missing =
+      Compare<hmi_apis::Common_Result::eType, EQ, ONE>(
+          ui_result_,
+          hmi_apis::Common_Result::INVALID_ENUM,
+          hmi_apis::Common_Result::SUCCESS,
+          hmi_apis::Common_Result::WARNINGS);
+
+  const bool is_vr_result_ok_or_missing =
+      Compare<hmi_apis::Common_Result::eType, EQ, ONE>(
+          vr_result_,
+          hmi_apis::Common_Result::INVALID_ENUM,
+          hmi_apis::Common_Result::SUCCESS,
+          hmi_apis::Common_Result::WARNINGS);
+
   const uint32_t cmd_id =
       (*message_)[strings::msg_params][strings::cmd_id].asUInt();
   smart_objects::SmartObject msg_params(smart_objects::SmartType_Map);
@@ -602,21 +608,15 @@ void AddCommandRequest::RemoveCommand() {
 
   app->RemoveCommand(cmd_id);
 
-  if (BothSend() && (IsInterfaceAwaited(HmiInterfaces::HMI_INTERFACE_VR) &&
-                     IsInterfaceAwaited(HmiInterfaces::HMI_INTERFACE_UI))) {
-    // in case we have send bth UI and VR and no one respond
-    // we have nothing to remove from HMI so no DeleteCommand expected
-    return;
-  }
-
-  if (BothSend() && IsInterfaceAwaited(HmiInterfaces::HMI_INTERFACE_VR)) {
+  if (ui_is_sent_ && is_ui_result_ok_or_missing) {
     SendHMIRequest(hmi_apis::FunctionID::UI_DeleteCommand, &msg_params);
   }
 
-  if (BothSend() && IsInterfaceAwaited(HmiInterfaces::HMI_INTERFACE_UI)) {
-    msg_params[strings::grammar_id] = app->get_grammar_id();
-    msg_params[strings::type] = hmi_apis::Common_VRCommandType::Command;
-    SendHMIRequest(hmi_apis::FunctionID::VR_DeleteCommand, &msg_params);
+  if (vr_is_sent_ && is_vr_result_ok_or_missing) {
+    smart_objects::SmartObject vr_msg_params = msg_params;
+    vr_msg_params[strings::grammar_id] = app->get_grammar_id();
+    vr_msg_params[strings::type] = hmi_apis::Common_VRCommandType::Command;
+    SendHMIRequest(hmi_apis::FunctionID::VR_DeleteCommand, &vr_msg_params);
   }
 }
 
diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/add_command_request_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/add_command_request_test.cc
index b4795dae21..d273ce8905 100644
--- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/add_command_request_test.cc
+++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/add_command_request_test.cc
@@ -167,8 +167,7 @@ class AddCommandRequestTest
   }
 
   void CheckOnTimeOutCommandDeletion(
-      const hmi_apis::FunctionID::eType incoming_cmd,
-      const hmi_apis::FunctionID::eType cmd_to_delete) {
+      const hmi_apis::FunctionID::eType incoming_cmd) {
     CreateBasicParamsVRRequest();
     CreateBasicParamsUIRequest();
     SmartObject& msg_params = (*msg_)[strings::msg_params];
@@ -209,9 +208,15 @@ class AddCommandRequestTest
     EXPECT_CALL(*mock_app_, RemoveCommand(kCmdId));
 
     EXPECT_CALL(mock_rpc_service_,
-                ManageHMICommand(HMIResultCodeIs(cmd_to_delete), _))
+                ManageHMICommand(
+                    HMIResultCodeIs(hmi_apis::FunctionID::UI_DeleteCommand), _))
+        .WillOnce(Return(true));
 
+    EXPECT_CALL(mock_rpc_service_,
+                ManageHMICommand(
+                    HMIResultCodeIs(hmi_apis::FunctionID::VR_DeleteCommand), _))
         .WillOnce(Return(true));
+
     SmartObjectSPtr response = std::make_shared<SmartObject>(SmartType_Map);
     (*response)[strings::msg_params][strings::info] = "info";
     EXPECT_CALL(
@@ -598,13 +603,11 @@ TEST_F(AddCommandRequestTest, OnEvent_VR_SUCCESS) {
 }
 
 TEST_F(AddCommandRequestTest, OnTimeOut_EXPECT_VR_DeleteCommand) {
-  CheckOnTimeOutCommandDeletion(hmi_apis::FunctionID::VR_AddCommand,
-                                hmi_apis::FunctionID::VR_DeleteCommand);
+  CheckOnTimeOutCommandDeletion(hmi_apis::FunctionID::VR_AddCommand);
 }
 
 TEST_F(AddCommandRequestTest, OnTimeOut_EXPECT_UI_DeleteCommand) {
-  CheckOnTimeOutCommandDeletion(hmi_apis::FunctionID::UI_AddCommand,
-                                hmi_apis::FunctionID::UI_DeleteCommand);
+  CheckOnTimeOutCommandDeletion(hmi_apis::FunctionID::UI_AddCommand);
 }
 
 TEST_F(AddCommandRequestTest, OnEvent_BothSend_SUCCESS) {
@@ -744,7 +747,7 @@ TEST_F(AddCommandRequestTest,
 
 TEST_F(
     AddCommandRequestTest,
-    OnEvent_UI_HmiResponseCodeIsGenericError_VR_HmiResponseCodeIsUnsupportedResourse_ExpectCommandRemoved) {
+    OnEvent_UI_HmiResponseCodeIsGenericError_VR_HmiResponseCodeIsUnsupportedResource_ExpectCommandRemoved) {
   CreateBasicParamsVRRequest();
   CreateBasicParamsUIRequest();
   SmartObject& params = (*msg_)[strings::params];
@@ -788,7 +791,7 @@ TEST_F(
 
 TEST_F(
     AddCommandRequestTest,
-    OnEvent_VR_HmiResponseCodeIsGenericError_UI_HmiResponseCodeIsUnsupportedResourse_ExpectCommandRemoved) {
+    OnEvent_VR_HmiResponseCodeIsGenericError_UI_HmiResponseCodeIsUnsupportedResource_ExpectCommandRemoved) {
   CreateBasicParamsVRRequest();
   CreateBasicParamsUIRequest();
   SmartObject& params = (*msg_)[strings::params];
@@ -833,7 +836,7 @@ TEST_F(
 
 TEST_F(
     AddCommandRequestTest,
-    OnEvent_UI_VR_HmiResponseCodeIsUnsupportedResourse_UI_NotAvailableInterfaceState_ExpectCommandRemoved) {
+    OnEvent_UI_VR_HmiResponseCodeIsUnsupportedResource_UI_NotAvailableInterfaceState_ExpectCommandRemoved) {
   CreateBasicParamsVRRequest();
   CreateBasicParamsUIRequest();
   SmartObject& params = (*msg_)[strings::params];
@@ -883,7 +886,7 @@ TEST_F(
 
 TEST_F(
     AddCommandRequestTest,
-    OnEvent_UI_VR_HmiResponseCodeIsUnsupportedResourse_VR_NotAvailableInterfaceState_ExpectCommandRemoved) {
+    OnEvent_UI_VR_HmiResponseCodeIsUnsupportedResource_VR_NotAvailableInterfaceState_ExpectCommandRemoved) {
   CreateBasicParamsVRRequest();
   CreateBasicParamsUIRequest();
   SmartObject& params = (*msg_)[strings::params];
@@ -1006,7 +1009,7 @@ TEST_F(
 }
 
 TEST_F(AddCommandRequestTest,
-       OnEvent_UI_EventWithNotSuccesResponseCode_ExpectVRCommandDelete) {
+       OnEvent_UI_EventWithNotSuccessResponseCode_ExpectVRCommandDelete) {
   CreateBasicParamsVRRequest();
   CreateBasicParamsUIRequest();
   SmartObject& params = (*msg_)[strings::params];
@@ -1048,7 +1051,7 @@ TEST_F(AddCommandRequestTest,
               ManageHMICommand(
                   HMIResultCodeIs(hmi_apis::FunctionID::VR_DeleteCommand), _))
       .WillOnce(Return(true));
-  EXPECT_CALL(*mock_app_, RemoveCommand(kCmdId)).Times(2);
+  EXPECT_CALL(*mock_app_, RemoveCommand(kCmdId));
   request_ptr->on_event(event_ui);
   request_ptr->on_event(event_vr);
 }
@@ -1091,7 +1094,7 @@ TEST_F(AddCommandRequestTest,
               ManageHMICommand(
                   HMIResultCodeIs(hmi_apis::FunctionID::UI_DeleteCommand), _))
       .WillOnce(Return(true));
-  EXPECT_CALL(*mock_app_, RemoveCommand(kCmdId)).Times(2);
+  EXPECT_CALL(*mock_app_, RemoveCommand(kCmdId));
   Event event_vr(hmi_apis::FunctionID::VR_AddCommand);
   MessageSharedPtr msg_vr = CreateMessage(SmartType_Map);
   (*msg_vr)[strings::params][hmi_response::code] =
@@ -1157,6 +1160,15 @@ TEST_F(AddCommandRequestTest, OnTimeOut_AppRemoveCommandCalled) {
       CreateCommand<AddCommandRequest>(msg_);
   request_ptr->Run();
   EXPECT_CALL(*mock_app_, RemoveCommand(kCmdId));
+  EXPECT_CALL(mock_rpc_service_,
+              ManageHMICommand(
+                  HMIResultCodeIs(hmi_apis::FunctionID::UI_DeleteCommand), _))
+      .WillOnce(Return(true));
+  EXPECT_CALL(mock_rpc_service_,
+              ManageHMICommand(
+                  HMIResultCodeIs(hmi_apis::FunctionID::VR_DeleteCommand), _))
+      .WillOnce(Return(true));
+
   SmartObjectSPtr response = std::make_shared<SmartObject>(SmartType_Map);
   (*response)[strings::msg_params][strings::info] = "info";
   EXPECT_CALL(
@@ -1166,6 +1178,7 @@ TEST_F(AddCommandRequestTest, OnTimeOut_AppRemoveCommandCalled) {
   EXPECT_CALL(mock_rpc_service_,
               ManageMobileCommand(
                   response, am::commands::Command::CommandSource::SOURCE_SDL));
+
   std::shared_ptr<RequestFromMobileImpl> base_class_request =
       static_cast<std::shared_ptr<RequestFromMobileImpl> >(request_ptr);
   base_class_request->OnTimeOut();