Fix wrong behavior of RCOnRemoteControlSettingsNotification (#3792)

* Fix wrong behavior of RCOnRemoteControlSettingsNotification

Looks like `kAllowed` parameter was swithing on/off the RC functionality
even if it was not present and previous value should be taken.
Processing of `kAllowed` and `kAccessMode` was split on two separate
and independent functions make it really isolated.
Processing of both parameters was aligned, so now core does not perform
extra actions if one of parameter is missing.

4 files changed, 98 insertions, 20 deletions

diff --git a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/include/rc_rpc_plugin/commands/hmi/rc_on_remote_control_settings_notification.h b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/include/rc_rpc_plugin/commands/hmi/rc_on_remote_control_settings_notification.h
index 82e0a8c13b..bb5e7a9b8f 100644
--- a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/include/rc_rpc_plugin/commands/hmi/rc_on_remote_control_settings_notification.h
+++ b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/include/rc_rpc_plugin/commands/hmi/rc_on_remote_control_settings_notification.h
@@ -76,6 +76,18 @@ class RCOnRemoteControlSettingsNotification
    * notifications
    */
   void DisallowRCFunctionality();
+
+  /**
+   * @brief Performs the set of actions depending on access mode param received
+   * in the message
+   */
+  void ProcessAccessModeParam();
+
+  /**
+   * @brief Performs the set of actions depending on allowed param received in
+   * the message
+   */
+  void ProcessAllowedParam();
 };
 }  // namespace commands
 }  // namespace rc_rpc_plugin
diff --git a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/include/rc_rpc_plugin/resource_allocation_manager.h b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/include/rc_rpc_plugin/resource_allocation_manager.h
index 623498dac7..db785e4a4f 100644
--- a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/include/rc_rpc_plugin/resource_allocation_manager.h
+++ b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/include/rc_rpc_plugin/resource_allocation_manager.h
@@ -209,8 +209,16 @@ class ResourceAllocationManager {
       NotificationTrigger::eType event,
       application_manager::ApplicationSharedPtr application) = 0;
 
+  /**
+   * @brief Returns current state of RC functionality
+   * @return current state of RC functionality
+   */
   virtual bool is_rc_enabled() const = 0;
 
+  /**
+   * @brief Sets current state of RC functionality to a new one
+   * @param value new RC functionality state
+   */
   virtual void set_rc_enabled(const bool value) = 0;
 
   /**
diff --git a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/commands/hmi/rc_on_remote_control_settings_notification.cc b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/commands/hmi/rc_on_remote_control_settings_notification.cc
index 860e18a105..580937bc8a 100644
--- a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/commands/hmi/rc_on_remote_control_settings_notification.cc
+++ b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/commands/hmi/rc_on_remote_control_settings_notification.cc
@@ -93,35 +93,50 @@ void RCOnRemoteControlSettingsNotification::Run() {
     return;
   }
 
-  hmi_apis::Common_RCAccessMode::eType access_mode =
-      hmi_apis::Common_RCAccessMode::INVALID_ENUM;
-  if ((*message_)[app_mngr::strings::msg_params].keyExists(
+  ProcessAccessModeParam();
+  ProcessAllowedParam();
+}
+
+void RCOnRemoteControlSettingsNotification::ProcessAccessModeParam() {
+  if (!(*message_)[app_mngr::strings::msg_params].keyExists(
           message_params::kAccessMode)) {
-    access_mode = static_cast<hmi_apis::Common_RCAccessMode::eType>(
-        (*message_)[app_mngr::strings::msg_params][message_params::kAccessMode]
-            .asUInt());
     SDL_LOG_DEBUG(
-        "Setting up access mode : " << AccessModeToString(access_mode));
-  } else {
-    access_mode = resource_allocation_manager_.GetAccessMode();
-    SDL_LOG_DEBUG("No access mode received. Using last known: "
-                  << AccessModeToString(access_mode));
+        "No access mode received. Using last known: "
+        << AccessModeToString(resource_allocation_manager_.GetAccessMode()));
+    return;
   }
+
+  const auto access_mode = static_cast<hmi_apis::Common_RCAccessMode::eType>(
+      (*message_)[app_mngr::strings::msg_params][message_params::kAccessMode]
+          .asUInt());
+  SDL_LOG_DEBUG("Setting up access mode : " << AccessModeToString(access_mode));
   resource_allocation_manager_.SetAccessMode(access_mode);
+}
+
+void RCOnRemoteControlSettingsNotification::ProcessAllowedParam() {
+  if (!(*message_)[app_mngr::strings::msg_params].keyExists(
+          message_params::kAllowed)) {
+    SDL_LOG_DEBUG("No allowed param received. Using last known: "
+                  << std::boolalpha
+                  << resource_allocation_manager_.is_rc_enabled());
+    return;
+  }
 
   const bool is_allowed =
       (*message_)[app_mngr::strings::msg_params][message_params::kAllowed]
           .asBool();
+
   if (is_allowed) {
     SDL_LOG_DEBUG("Allowing RC Functionality");
     resource_allocation_manager_.set_rc_enabled(true);
-  } else {
-    SDL_LOG_DEBUG("Disallowing RC Functionality");
-    DisallowRCFunctionality();
-    resource_allocation_manager_.ResetAllAllocations();
-    resource_allocation_manager_.set_rc_enabled(false);
-    rc_consent_manager_.RemoveAllConsents();
+    return;
   }
+
+  SDL_LOG_DEBUG("Disallowing RC Functionality");
+  DisallowRCFunctionality();
+  resource_allocation_manager_.ResetAllAllocations();
+  resource_allocation_manager_.set_rc_enabled(false);
+  rc_consent_manager_.RemoveAllConsents();
 }
 
 }  // namespace commands
diff --git a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/test/commands/on_remote_control_settings_test.cc b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/test/commands/on_remote_control_settings_test.cc
index 7907013fa3..5d569e93d2 100644
--- a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/test/commands/on_remote_control_settings_test.cc
+++ b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/test/commands/on_remote_control_settings_test.cc
@@ -117,18 +117,61 @@ class RCOnRemoteControlSettingsNotificationTest
 };
 
 TEST_F(RCOnRemoteControlSettingsNotificationTest,
-       Run_Allowed_SetAccessMode) {  // Arrange
+       Run_Allowed_MissedAccessMode) {  // Arrange
   MessageSharedPtr mobile_message = CreateBasicMessage();
   (*mobile_message)[application_manager::strings::msg_params]
                    [message_params::kAllowed] = true;
 
   // Expectations
+  EXPECT_CALL(mock_allocation_manager_, SetAccessMode(_)).Times(0);
+  EXPECT_CALL(mock_allocation_manager_, set_rc_enabled(true));
 
-  ON_CALL(mock_allocation_manager_, GetAccessMode())
-      .WillByDefault(Return(hmi_apis::Common_RCAccessMode::ASK_DRIVER));
+  // Act
+  std::shared_ptr<
+      rc_rpc_plugin::commands::RCOnRemoteControlSettingsNotification>
+      command = CreateRCCommand<
+          rc_rpc_plugin::commands::RCOnRemoteControlSettingsNotification>(
+          mobile_message);
 
+  command->Run();
+}
+
+TEST_F(RCOnRemoteControlSettingsNotificationTest,
+       Run_AccessMode_MissedAllowed) {  // Arrange
+  MessageSharedPtr mobile_message = CreateBasicMessage();
+  (*mobile_message)[application_manager::strings::msg_params]
+                   [message_params::kAccessMode] =
+                       hmi_apis::Common_RCAccessMode::ASK_DRIVER;
+
+  // Expectations
+  EXPECT_CALL(mock_allocation_manager_, set_rc_enabled(_)).Times(0);
   EXPECT_CALL(mock_allocation_manager_,
               SetAccessMode(hmi_apis::Common_RCAccessMode::ASK_DRIVER));
+
+  // Act
+  std::shared_ptr<
+      rc_rpc_plugin::commands::RCOnRemoteControlSettingsNotification>
+      command = CreateRCCommand<
+          rc_rpc_plugin::commands::RCOnRemoteControlSettingsNotification>(
+          mobile_message);
+
+  command->Run();
+}
+
+TEST_F(RCOnRemoteControlSettingsNotificationTest,
+       Run_AccessModeAndAllowed_BothPresent) {  // Arrange
+  MessageSharedPtr mobile_message = CreateBasicMessage();
+  (*mobile_message)[application_manager::strings::msg_params]
+                   [message_params::kAllowed] = true;
+  (*mobile_message)[application_manager::strings::msg_params]
+                   [message_params::kAccessMode] =
+                       hmi_apis::Common_RCAccessMode::ASK_DRIVER;
+
+  // Expectations
+  EXPECT_CALL(mock_allocation_manager_, set_rc_enabled(true));
+  EXPECT_CALL(mock_allocation_manager_,
+              SetAccessMode(hmi_apis::Common_RCAccessMode::ASK_DRIVER));
+
   // Act
   std::shared_ptr<
       rc_rpc_plugin::commands::RCOnRemoteControlSettingsNotification>