Address comments

6 files changed, 82 insertions, 51 deletions

diff --git a/src/components/application_manager/include/application_manager/rpc_passing_handler.h b/src/components/application_manager/include/application_manager/rpc_passing_handler.h
index 55662c1537..45fe4cad2f 100644
--- a/src/components/application_manager/include/application_manager/rpc_passing_handler.h
+++ b/src/components/application_manager/include/application_manager/rpc_passing_handler.h
@@ -104,6 +104,9 @@ class RPCPassingHandler {
   void ForwardResponseToMobile(uint32_t correlation_id,
                                smart_objects::SmartObject response_message);
   void PopulateRPCRequestQueue(smart_objects::SmartObject request_message);
+  bool ExtractRPCParams(const smart_objects::SmartObject& s_map,
+                        const ApplicationSharedPtr app,
+                        const std::string& function_id_str);
 
   AppServiceManager& app_service_manager_;
   ApplicationManager& app_manager_;
diff --git a/src/components/application_manager/include/application_manager/rpc_service_impl.h b/src/components/application_manager/include/application_manager/rpc_service_impl.h
index 516b8abb8b..98aad1a90e 100644
--- a/src/components/application_manager/include/application_manager/rpc_service_impl.h
+++ b/src/components/application_manager/include/application_manager/rpc_service_impl.h
@@ -135,6 +135,9 @@ class RPCServiceImpl : public RPCService,
                           const bool allow_unknown_parameters = false);
   hmi_apis::HMI_API& hmi_so_factory();
   mobile_apis::MOBILE_API& mobile_so_factory();
+  void CheckSourceForUnsupportedRequest(
+      const commands::MessageSharedPtr message,
+      commands::Command::CommandSource source);
 
   ApplicationManager& app_manager_;
   request_controller::RequestController& request_ctrl_;
diff --git a/src/components/application_manager/src/rpc_passing_handler.cc b/src/components/application_manager/src/rpc_passing_handler.cc
index 40097da962..4780932fea 100644
--- a/src/components/application_manager/src/rpc_passing_handler.cc
+++ b/src/components/application_manager/src/rpc_passing_handler.cc
@@ -93,34 +93,13 @@ bool RPCPassingHandler::CanHandleFunctionID(int32_t function_id) {
   return false;
 }
 
-bool RPCPassingHandler::IsPassthroughAllowed(
-    smart_objects::SmartObject rpc_message) {
+bool RPCPassingHandler::ExtractRPCParams(
+    const smart_objects::SmartObject& s_map,
+    const ApplicationSharedPtr app,
+    const std::string& function_id_str) {
   LOG4CXX_AUTO_TRACE(logger_);
-  mobile_api::FunctionID::eType function_id =
-      static_cast<mobile_api::FunctionID::eType>(
-          rpc_message[strings::params][strings::function_id].asInt());
-  uint32_t connection_key =
-      rpc_message[strings::params][strings::connection_key].asUInt();
-  std::string function_id_str =
-      MessageHelper::StringifiedFunctionID(function_id);
-  ApplicationSharedPtr app = app_manager_.application(connection_key);
-  PolicyHandlerInterface& policy_handler = app_manager_.GetPolicyHandler();
-  if (!app) {
-    return false;
-  } else if (function_id_str.empty()) {
-    // Unknown RPC, just do basic revoked and user consent checks
-    std::string device_mac;
-    app_manager_.connection_handler().get_session_observer().GetDataOnDeviceID(
-        app->device(), NULL, NULL, &device_mac, NULL);
-    return policy_handler.UnknownRPCPassthroughAllowed(app->policy_app_id()) &&
-           !policy_handler.IsApplicationRevoked(app->policy_app_id()) &&
-           policy::kDeviceAllowed ==
-               app_manager_.GetUserConsentForDevice(device_mac);
-  }
-
   RPCParams params;
 
-  const smart_objects::SmartObject& s_map = rpc_message[strings::msg_params];
   if (smart_objects::SmartType_Map == s_map.getType()) {
     smart_objects::SmartMap::const_iterator iter = s_map.map_begin();
     smart_objects::SmartMap::const_iterator iter_end = s_map.map_end();
@@ -150,6 +129,38 @@ bool RPCPassingHandler::IsPassthroughAllowed(
   return true;
 }
 
+bool RPCPassingHandler::IsPassthroughAllowed(
+    smart_objects::SmartObject rpc_message) {
+  LOG4CXX_AUTO_TRACE(logger_);
+  uint32_t connection_key =
+      rpc_message[strings::params][strings::connection_key].asUInt();
+  ApplicationSharedPtr app = app_manager_.application(connection_key);
+  if (!app) {
+    return false;
+  }
+
+  mobile_api::FunctionID::eType function_id =
+      static_cast<mobile_api::FunctionID::eType>(
+          rpc_message[strings::params][strings::function_id].asInt());
+  std::string function_id_str =
+      MessageHelper::StringifiedFunctionID(function_id);
+  PolicyHandlerInterface& policy_handler = app_manager_.GetPolicyHandler();
+
+  if (function_id_str.empty()) {
+    // Unknown RPC, just do basic revoked and user consent checks
+    std::string device_mac;
+    app_manager_.connection_handler().get_session_observer().GetDataOnDeviceID(
+        app->device(), NULL, NULL, &device_mac, NULL);
+    return policy_handler.UnknownRPCPassthroughAllowed(app->policy_app_id()) &&
+           !policy_handler.IsApplicationRevoked(app->policy_app_id()) &&
+           policy::kDeviceAllowed ==
+               app_manager_.GetUserConsentForDevice(device_mac);
+  }
+
+  return ExtractRPCParams(
+      rpc_message[strings::msg_params], app, function_id_str);
+}
+
 bool RPCPassingHandler::RPCPassThrough(smart_objects::SmartObject rpc_message) {
   LOG4CXX_AUTO_TRACE(logger_);
 
diff --git a/src/components/application_manager/src/rpc_service_impl.cc b/src/components/application_manager/src/rpc_service_impl.cc
index c55a16bc5f..daa8cb2ac5 100644
--- a/src/components/application_manager/src/rpc_service_impl.cc
+++ b/src/components/application_manager/src/rpc_service_impl.cc
@@ -127,30 +127,7 @@ bool RPCServiceImpl::ManageMobileCommand(
       app_manager_.GetPluginManager().FindPluginToProcess(function_id, source);
   if (!plugin) {
     LOG4CXX_WARN(logger_, "Failed to find plugin : " << plugin.error());
-    int32_t message_type =
-        (*(message.get()))[strings::params][strings::message_type].asInt();
-    if ((source == commands::Command::CommandSource::SOURCE_MOBILE &&
-         kRequest == message_type) ||
-        (source == commands::Command::CommandSource::SOURCE_SDL &&
-         kResponse == message_type)) {
-      smart_objects::SmartObjectSPtr response =
-          MessageHelper::CreateNegativeResponse(
-              connection_key,
-              static_cast<int32_t>(function_id),
-              correlation_id,
-              0);
-
-      // Since we are dealing with an unknown RPC, there is no schema attached
-      // to the message, so we have to convert the result to string directly
-      std::string result_code;
-      smart_objects::EnumConversionHelper<mobile_apis::Result::eType>::
-          EnumToString(mobile_apis::Result::UNSUPPORTED_REQUEST, &result_code);
-      (*response)[strings::msg_params][strings::result_code] = result_code;
-      (*response)[strings::msg_params][strings::info] =
-          "Module does not recognize this function id";
-
-      SendMessageToMobile(response);
-    }
+    CheckSourceForUnsupportedRequest(message, source);
     return false;
   }
   application_manager::CommandFactory& factory = (*plugin).GetCommandFactory();
@@ -738,6 +715,43 @@ mobile_apis::MOBILE_API& RPCServiceImpl::mobile_so_factory() {
   return mobile_so_factory_;
 }
 
+void RPCServiceImpl::CheckSourceForUnsupportedRequest(
+    const commands::MessageSharedPtr message,
+    commands::Command::CommandSource source) {
+  int32_t message_type =
+      (*(message.get()))[strings::params][strings::message_type].asInt();
+  uint32_t correlation_id =
+      (*message)[strings::params].keyExists(strings::correlation_id)
+          ? (*message)[strings::params][strings::correlation_id].asUInt()
+          : 0;
+  const uint32_t connection_key = static_cast<uint32_t>(
+      (*message)[strings::params][strings::connection_key].asUInt());
+  mobile_apis::FunctionID::eType function_id =
+      static_cast<mobile_apis::FunctionID::eType>(
+          (*message)[strings::params][strings::function_id].asInt());
+  if ((source == commands::Command::CommandSource::SOURCE_MOBILE &&
+       kRequest == message_type) ||
+      (source == commands::Command::CommandSource::SOURCE_SDL &&
+       kResponse == message_type)) {
+    smart_objects::SmartObjectSPtr response =
+        MessageHelper::CreateNegativeResponse(connection_key,
+                                              static_cast<int32_t>(function_id),
+                                              correlation_id,
+                                              0);
+
+    // Since we are dealing with an unknown RPC, there is no schema attached
+    // to the message, so we have to convert the result to string directly
+    std::string result_code;
+    smart_objects::EnumConversionHelper<mobile_apis::Result::eType>::
+        EnumToString(mobile_apis::Result::UNSUPPORTED_REQUEST, &result_code);
+    (*response)[strings::msg_params][strings::result_code] = result_code;
+    (*response)[strings::msg_params][strings::info] =
+        "Module does not recognize this function id";
+
+    SendMessageToMobile(response);
+  }
+}
+
 }  // namespace rpc_service
 
 }  // namespace application_manager
diff --git a/src/components/policy/policy_external/src/cache_manager.cc b/src/components/policy/policy_external/src/cache_manager.cc
index bc81b0c57d..4a738aa6ed 100644
--- a/src/components/policy/policy_external/src/cache_manager.cc
+++ b/src/components/policy/policy_external/src/cache_manager.cc
@@ -1548,7 +1548,7 @@ bool CacheManager::UnknownRPCPassthroughAllowed(
   policy_table::ApplicationPolicies::const_iterator policy_iter =
       policies.find(policy_app_id);
   if (policies.end() != policy_iter) {
-    auto app_policy = (*policy_iter).second;
+    const auto app_policy = (*policy_iter).second;
     if (app_policy.allow_unknown_rpc_passthrough.is_initialized()) {
       return *(app_policy.allow_unknown_rpc_passthrough);
     }
diff --git a/src/components/policy/policy_regular/src/cache_manager.cc b/src/components/policy/policy_regular/src/cache_manager.cc
index 54a571609f..991c6a1363 100644
--- a/src/components/policy/policy_regular/src/cache_manager.cc
+++ b/src/components/policy/policy_regular/src/cache_manager.cc
@@ -843,7 +843,7 @@ bool CacheManager::UnknownRPCPassthroughAllowed(
   policy_table::ApplicationPolicies::const_iterator policy_iter =
       policies.find(policy_app_id);
   if (policies.end() != policy_iter) {
-    auto app_policy = (*policy_iter).second;
+    const auto app_policy = (*policy_iter).second;
     if (app_policy.allow_unknown_rpc_passthrough.is_initialized()) {
       return *(app_policy.allow_unknown_rpc_passthrough);
     }