Fixes responses 'success' value in case of UNSUPPORTED_RESOURCE (#1576)

* Fixes 'success' value in case of UNSUPPORTED_RESOURCE

There is only one case for 'success' to have 'false' with UNSUPPORTED_RESOURCE
- when there is single-interface RPC comes and that specific interface is
not available i.e. <interface>.IsReady returned 'false'
Changes done for base class and unit tests with wrong expectations.

* Fix response to mobile result

Fixed result to mobile in case of GENERIC_ERROR from hmi and iface does
not response

* Fix ut tests according to code changes

* Fix regression

7 files changed, 77 insertions, 41 deletions

diff --git a/src/components/application_manager/include/application_manager/commands/command_request_impl.h b/src/components/application_manager/include/application_manager/commands/command_request_impl.h
index aa3c216d4d..614f86a6f1 100644
--- a/src/components/application_manager/include/application_manager/commands/command_request_impl.h
+++ b/src/components/application_manager/include/application_manager/commands/command_request_impl.h
@@ -49,7 +49,7 @@ struct ResponseInfo {
       , interface_state(HmiInterfaces::STATE_NOT_RESPONSE)
       , is_ok(false)
       , is_unsupported_resource(false)
-      , is_invalid_enum(false) {}
+      , is_not_used(false) {}
   ResponseInfo(hmi_apis::Common_Result::eType result,
                HmiInterfaces::InterfaceID interface)
       : result_code(result)
@@ -57,13 +57,13 @@ struct ResponseInfo {
       , interface_state(HmiInterfaces::STATE_NOT_RESPONSE)
       , is_ok(false)
       , is_unsupported_resource(false)
-      , is_invalid_enum(false) {}
+      , is_not_used(false) {}
   hmi_apis::Common_Result::eType result_code;
   HmiInterfaces::InterfaceID interface;
   HmiInterfaces::InterfaceState interface_state;
   bool is_ok;
   bool is_unsupported_resource;
-  bool is_invalid_enum;
+  bool is_not_used;
 };
 
 namespace NsSmart = NsSmartDeviceLink::NsSmartObjects;
diff --git a/src/components/application_manager/src/commands/command_request_impl.cc b/src/components/application_manager/src/commands/command_request_impl.cc
index 3f9a1d13b3..efd8c8f042 100644
--- a/src/components/application_manager/src/commands/command_request_impl.cc
+++ b/src/components/application_manager/src/commands/command_request_impl.cc
@@ -105,18 +105,31 @@ const std::string CreateInfoForUnsupportedResult(
 }
 
 bool CheckResultCode(const ResponseInfo& first, const ResponseInfo& second) {
-  if (first.is_ok && second.is_unsupported_resource &&
-      second.interface_state == HmiInterfaces::STATE_NOT_AVAILABLE) {
-    return true;
+  if (!first.is_ok && !second.is_ok) {
+    return false;
   }
-  return false;
+
+  if (!first.is_ok && second.is_not_used) {
+    return false;
+  }
+
+  if (!second.is_ok && first.is_not_used) {
+    return false;
+  }
+
+  if (first.is_unsupported_resource && second.is_not_used &&
+      HmiInterfaces::STATE_NOT_AVAILABLE == first.interface_state) {
+    return false;
+  }
+
+  return true;
 }
 
 bool IsResultCodeUnsupported(const ResponseInfo& first,
                              const ResponseInfo& second) {
-  return ((first.is_ok || first.is_invalid_enum) &&
+  return ((first.is_ok || first.is_not_used) &&
           second.is_unsupported_resource) ||
-         ((second.is_ok || second.is_invalid_enum) &&
+         ((second.is_ok || second.is_not_used) &&
           first.is_unsupported_resource) ||
          (first.is_unsupported_resource && second.is_unsupported_resource);
 }
@@ -204,6 +217,7 @@ void CommandRequestImpl::SendResponse(
     const mobile_apis::Result::eType& result_code,
     const char* info,
     const smart_objects::SmartObject* response_params) {
+  LOG4CXX_AUTO_TRACE(logger_);
   {
     sync_primitives::AutoLock auto_lock(state_lock_);
     if (kTimedOut == current_state_) {
@@ -777,6 +791,7 @@ bool CommandRequestImpl::PrepareResultForMobileResponse(
 
 bool CommandRequestImpl::PrepareResultForMobileResponse(
     ResponseInfo& out_first, ResponseInfo& out_second) const {
+  LOG4CXX_AUTO_TRACE(logger_);
   using namespace helpers;
 
   out_first.is_ok = Compare<hmi_apis::Common_Result::eType, EQ, ONE>(
@@ -795,10 +810,10 @@ bool CommandRequestImpl::PrepareResultForMobileResponse(
       hmi_apis::Common_Result::RETRY,
       hmi_apis::Common_Result::SAVED);
 
-  out_first.is_invalid_enum =
+  out_first.is_not_used =
       hmi_apis::Common_Result::INVALID_ENUM == out_first.result_code;
 
-  out_second.is_invalid_enum =
+  out_second.is_not_used =
       hmi_apis::Common_Result::INVALID_ENUM == out_second.result_code;
 
   out_first.is_unsupported_resource =
@@ -810,13 +825,15 @@ bool CommandRequestImpl::PrepareResultForMobileResponse(
   out_first.interface_state =
       application_manager_.hmi_interfaces().GetInterfaceState(
           out_first.interface);
+
   out_second.interface_state =
       application_manager_.hmi_interfaces().GetInterfaceState(
           out_second.interface);
 
   bool result = (out_first.is_ok && out_second.is_ok) ||
-                (out_second.is_invalid_enum && out_first.is_ok) ||
-                (out_first.is_invalid_enum && out_second.is_ok);
+                (out_second.is_not_used && out_first.is_ok) ||
+                (out_first.is_not_used && out_second.is_ok);
+
   result = result || CheckResultCode(out_first, out_second);
   result = result || CheckResultCode(out_second, out_first);
   return result;
@@ -835,6 +852,7 @@ void CommandRequestImpl::GetInfo(
 
 mobile_apis::Result::eType CommandRequestImpl::PrepareResultCodeForResponse(
     const ResponseInfo& first, const ResponseInfo& second) {
+  LOG4CXX_AUTO_TRACE(logger_);
   mobile_apis::Result::eType result_code = mobile_apis::Result::INVALID_ENUM;
   if (IsResultCodeUnsupported(first, second)) {
     result_code = mobile_apis::Result::UNSUPPORTED_RESOURCE;
diff --git a/src/components/application_manager/src/commands/mobile/alert_request.cc b/src/components/application_manager/src/commands/mobile/alert_request.cc
index 3288870b92..53515a810a 100644
--- a/src/components/application_manager/src/commands/mobile/alert_request.cc
+++ b/src/components/application_manager/src/commands/mobile/alert_request.cc
@@ -219,7 +219,7 @@ bool AlertRequest::PrepareResponseParameters(
     result = false;
   }
   result_code = mobile_apis::Result::WARNINGS;
-  if ((ui_alert_info.is_ok || ui_alert_info.is_invalid_enum) &&
+  if ((ui_alert_info.is_ok || ui_alert_info.is_not_used) &&
       tts_alert_info.is_unsupported_resource &&
       HmiInterfaces::STATE_AVAILABLE == tts_alert_info.interface_state) {
     tts_response_info_ = "Unsupported phoneme type sent in a prompt";
@@ -230,6 +230,10 @@ bool AlertRequest::PrepareResponseParameters(
   result_code = PrepareResultCodeForResponse(ui_alert_info, tts_alert_info);
   info = MergeInfos(
       ui_alert_info, ui_response_info_, tts_alert_info, tts_response_info_);
+  // Mobile Alert request is successful when UI_Alert is successful
+  if (is_ui_alert_sent_ && !ui_alert_info.is_ok) {
+    return false;
+  }
   return result;
 }
 
diff --git a/src/components/application_manager/src/commands/mobile/perform_interaction_request.cc b/src/components/application_manager/src/commands/mobile/perform_interaction_request.cc
index 007440e8e6..b199e652a6 100644
--- a/src/components/application_manager/src/commands/mobile/perform_interaction_request.cc
+++ b/src/components/application_manager/src/commands/mobile/perform_interaction_request.cc
@@ -143,11 +143,10 @@ void PerformInteractionRequest::Run() {
     }
   }
 
-  if (choice_set_id_list_length &&
-      (!CheckChoiceIDFromRequest(
-           app,
-           choice_set_id_list_length,
-           msg_params[strings::interaction_choice_set_id_list]))) {
+  if (!CheckChoiceIDFromRequest(
+          app,
+          choice_set_id_list_length,
+          msg_params[strings::interaction_choice_set_id_list])) {
     LOG4CXX_ERROR(logger_,
                   "PerformInteraction has choice sets with "
                   "duplicated IDs or application does not have choice sets");
diff --git a/src/components/application_manager/test/commands/mobile/alert_maneuver_request_test.cc b/src/components/application_manager/test/commands/mobile/alert_maneuver_request_test.cc
index ee0662e5c6..9a101e6352 100644
--- a/src/components/application_manager/test/commands/mobile/alert_maneuver_request_test.cc
+++ b/src/components/application_manager/test/commands/mobile/alert_maneuver_request_test.cc
@@ -59,6 +59,7 @@ namespace alert_maneuver_request {
 using ::testing::_;
 using ::testing::Return;
 using ::testing::ReturnRef;
+using ::testing::Mock;
 namespace am = ::application_manager;
 using am::commands::AlertManeuverRequest;
 using am::commands::MessageSharedPtr;
@@ -70,6 +71,9 @@ typedef SharedPtr<AlertManeuverRequest> CommandPtr;
 class AlertManeuverRequestTest
     : public CommandRequestTest<CommandsTestMocks::kIsNice> {
  public:
+  AlertManeuverRequestTest()
+      : mock_message_helper_(*MockMessageHelper::message_helper_mock()) {}
+
   void CheckExpectations(const hmi_apis::Common_Result::eType hmi_response,
                          const mobile_apis::Result::eType mobile_response,
                          const am::HmiInterfaces::InterfaceState state,
@@ -87,10 +91,10 @@ class AlertManeuverRequestTest
     MockAppPtr mock_app(CreateMockApp());
     ON_CALL(app_mngr_, application(_)).WillByDefault(Return(mock_app));
 
-    MockMessageHelper* mock_message_helper =
-        MockMessageHelper::message_helper_mock();
-    EXPECT_CALL(*mock_message_helper, HMIToMobileResult(_))
-        .WillOnce(Return(mobile_apis::Result::UNSUPPORTED_RESOURCE));
+    if (hmi_apis::Common_Result::UNSUPPORTED_RESOURCE != hmi_response) {
+      EXPECT_CALL(mock_message_helper_, HMIToMobileResult(hmi_response))
+          .WillOnce(Return(mobile_response));
+    }
 
     EXPECT_CALL(mock_hmi_interfaces_, GetInterfaceState(_))
         .WillRepeatedly(Return(state));
@@ -112,7 +116,16 @@ class AlertManeuverRequestTest
         static_cast<int32_t>(mobile_response));
   }
 
+  void SetUp() OVERRIDE {
+    Mock::VerifyAndClearExpectations(&mock_message_helper_);
+  }
+
+  void TearDown() OVERRIDE {
+    Mock::VerifyAndClearExpectations(&mock_message_helper_);
+  }
+
  protected:
+  MockMessageHelper& mock_message_helper_;
   NiceMock<policy_test::MockPolicyHandlerInterface> policy_interface_;
 };
 
@@ -237,30 +250,30 @@ TEST_F(AlertManeuverRequestTest, OnEvent_ReceivedUnknownEvent_UNSUCCESS) {
                     .asInt()));
 }
 
-TEST_F(AlertManeuverRequestTest, OnEvent_UNSUPPORTED_RESOURCE_Case1) {
+TEST_F(AlertManeuverRequestTest, OnEvent_SUCCESS) {
   CheckExpectations(hmi_apis::Common_Result::SUCCESS,
-                    mobile_apis::Result::UNSUPPORTED_RESOURCE,
+                    mobile_apis::Result::SUCCESS,
                     am::HmiInterfaces::STATE_AVAILABLE,
                     true);
 }
 
-TEST_F(AlertManeuverRequestTest, OnEvent_UNSUPPORTED_RESOURCE_Case2) {
-  CheckExpectations(hmi_apis::Common_Result::SUCCESS,
+TEST_F(AlertManeuverRequestTest, OnEvent_UNSUPPORTED_RESOURCE) {
+  CheckExpectations(hmi_apis::Common_Result::UNSUPPORTED_RESOURCE,
                     mobile_apis::Result::UNSUPPORTED_RESOURCE,
-                    am::HmiInterfaces::STATE_NOT_AVAILABLE,
-                    true);
+                    am::HmiInterfaces::STATE_AVAILABLE,
+                    false);
 }
 
-TEST_F(AlertManeuverRequestTest, OnEvent_UNSUPPORTED_RESOURCE_Case3) {
-  CheckExpectations(hmi_apis::Common_Result::SUCCESS,
-                    mobile_apis::Result::UNSUPPORTED_RESOURCE,
+TEST_F(AlertManeuverRequestTest, OnEvent_WARNINGS) {
+  CheckExpectations(hmi_apis::Common_Result::WARNINGS,
+                    mobile_apis::Result::WARNINGS,
                     am::HmiInterfaces::STATE_NOT_RESPONSE,
                     true);
 }
 
-TEST_F(AlertManeuverRequestTest, OnEvent_UNSUPPORTED_RESOURCE_Case4) {
+TEST_F(AlertManeuverRequestTest, OnEvent_GENERIC_ERROR) {
   CheckExpectations(hmi_apis::Common_Result::GENERIC_ERROR,
-                    mobile_apis::Result::UNSUPPORTED_RESOURCE,
+                    mobile_apis::Result::GENERIC_ERROR,
                     am::HmiInterfaces::STATE_NOT_RESPONSE,
                     false);
 }
diff --git a/src/components/application_manager/test/commands/mobile/change_registration_test.cc b/src/components/application_manager/test/commands/mobile/change_registration_test.cc
index 2fd43a6353..cbf8ce1abe 100644
--- a/src/components/application_manager/test/commands/mobile/change_registration_test.cc
+++ b/src/components/application_manager/test/commands/mobile/change_registration_test.cc
@@ -80,7 +80,6 @@ namespace strings = ::application_manager::strings;
 namespace hmi_response = ::application_manager::hmi_response;
 
 namespace {
-const int32_t kCommandId = 1;
 const uint32_t kAppId = 1u;
 const uint32_t kCmdId = 1u;
 const uint32_t kConnectionKey = 2u;
@@ -391,7 +390,7 @@ TEST_F(ChangeRegistrationRequestTest,
   CheckExpectations(hmi_apis::Common_Result::UNSUPPORTED_RESOURCE,
                     mobile_apis::Result::UNSUPPORTED_RESOURCE,
                     am::HmiInterfaces::STATE_NOT_RESPONSE,
-                    false);
+                    true);
 }
 
 TEST_F(ChangeRegistrationRequestTest,
@@ -399,7 +398,7 @@ TEST_F(ChangeRegistrationRequestTest,
   CheckExpectations(hmi_apis::Common_Result::UNSUPPORTED_RESOURCE,
                     mobile_apis::Result::UNSUPPORTED_RESOURCE,
                     am::HmiInterfaces::STATE_AVAILABLE,
-                    false);
+                    true);
 }
 
 TEST_F(ChangeRegistrationRequestTest,
@@ -407,7 +406,7 @@ TEST_F(ChangeRegistrationRequestTest,
   CheckExpectations(hmi_apis::Common_Result::UNSUPPORTED_RESOURCE,
                     mobile_apis::Result::UNSUPPORTED_RESOURCE,
                     am::HmiInterfaces::STATE_AVAILABLE,
-                    false,
+                    true,
                     hmi_apis::Common_Result::SUCCESS,
                     hmi_apis::Common_Result::SUCCESS);
 }
diff --git a/src/components/application_manager/test/commands/mobile/set_media_clock_timer_test.cc b/src/components/application_manager/test/commands/mobile/set_media_clock_timer_test.cc
index 3056d0e9fc..d91087acae 100644
--- a/src/components/application_manager/test/commands/mobile/set_media_clock_timer_test.cc
+++ b/src/components/application_manager/test/commands/mobile/set_media_clock_timer_test.cc
@@ -329,10 +329,11 @@ TEST_F(SetMediaClockRequestTest, OnEvent_Success) {
   MessageSharedPtr msg = CreateMessage();
   (*msg)[am::strings::params][am::hmi_response::code] =
       mobile_apis::Result::SUCCESS;
-  (*msg)[am::strings::msg_params] = SmartObject(smart_objects::SmartType_Null);
+  (*msg)[am::strings::msg_params] = SmartObject(smart_objects::SmartType_Map);
 
-  SharedPtr<SetMediaClockRequest> command(
-      CreateCommand<SetMediaClockRequest>(msg));
+  EXPECT_CALL(mock_message_helper_,
+              HMIToMobileResult(hmi_apis::Common_Result::SUCCESS))
+      .WillOnce(Return(mobile_apis::Result::SUCCESS));
 
   EXPECT_CALL(app_mngr_, ManageMobileCommand(_, _));
 
@@ -342,6 +343,8 @@ TEST_F(SetMediaClockRequestTest, OnEvent_Success) {
   Event event(hmi_apis::FunctionID::UI_SetMediaClockTimer);
   event.set_smart_object(*msg);
 
+  SharedPtr<SetMediaClockRequest> command(
+      CreateCommand<SetMediaClockRequest>(msg));
   command->on_event(event);
 }